Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Commit #484694 introduced some inconsistency to IsValidGitRepo - which now returns either an array of errors or TRUE. Presently the two calls to this method check if (!empty($repo->IsValidGitRepo())), which always returns true now (essentially treating all repos as invalid).
Here are two solutions:
A - change the method to IsInvalidRepo - I think this is preferable, since it makes the return values more consistent with the method name.
B - alter the conditions dependent on IsValidRepo to check for an array.
Patches attached!
Comments
Comment #1
sdboyer commentedMan, that's the second problem with that commit. I knew I should have reviewed it more thoroughly...I'd just had it sitting locally for a week and thought I could throw it in.
Frankly, I don't like either solution here. The initial changes I made were going towards returning a simple boolean, and I'd like them to continue in that direction. Moreover, "IsInvalidRepo" is a negated question, which tends to be more confusing. I'd like to keep the method as-is.
Comment #2
jonathan webb commentedRevised patch "C" - isValidGitRepo() only returns a boolean. Calling functions responsible for error handling. Porkbarrelling a minor clarification of bare repo to the "root" form field's description.
Comment #3
sdboyer commentedLooks good. Committed. Thanks!