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!

CommentFileSizeAuthor
#2 versioncontrol_git-isValidGitRepo-solutionC.patch2.99 KBjonathan webb
versioncontrol_git-isValidGitRepo-solutionB.patch1.01 KBjonathan webb
versioncontrol_git-isValidGitRepo-solutionA.patch2.11 KBjonathan webb

Comments

sdboyer’s picture

sdboyer commented

Man, 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.

jonathan webb’s picture

jonathan webb commented
Assigned: Unassigned » jonathan webb
StatusFileSize
new versioncontrol_git-isValidGitRepo-solutionC.patch2.99 KB

Revised 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.

sdboyer’s picture

sdboyer commented
Status: Needs review » Fixed

Looks good. Committed. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -git phase 2, -git sprint 9

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +git phase 2, +git sprint 9

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.

  • Commit dd37e48 on 6.x-2.x, fix-invalid-default-branches, fullsync-memory by sdboyer: 
    Issue #1032702 by Jonathan Webb: make VersioncontrolGitRepository::...