Lidong Yan <yldhome2d2@xxxxxxxxx> writes: > I've been reading through the git code from the beginning. This > patch series fixes some NEEDSWORKs and cleans up some unnecessary > uses of the_repository that I came across. FYI, when we have "NEEDSWORK: do X", the intention is often "we haven't spent enough brain cycles when we wrote this comment, so the first step is to evaluate if doing X is a sensible thing in the first place, and only if that is the case, do X". > The first commit replace the use of the_repository to run_builtin()'s > argument repo. Since each caller pass the_repository to run_builtin(), > this replacement is safe. That change is safe. I'd rather see the comment left intact or reverted to the original shape to clarify what code the comment applies to (see the other message). > The second commit takes care of a NEEDSWORK in setup_git_directory_gently() > we now properly error out if we hit a .git that is not a file or directory > when looking for the .git. We used to just ignore and keep going to check the parent directory, right? Now we would error out when .git is a FIFO or device or any other random things. Is a bit of behaviour change, but I am not sure if it is worth doing. As finding these weird non-file things in your working tree and naming them ".git" is extremely rare and useless (from Git's point of view), I suspect that the user is deliberately doing so for whatever reason they have, so it smells like this change has very little chance to detect a real problem with a larger chance to break a set-up that was deliberately done by the end-user. I dunno. Thanks.
