On Wed, Sep 16, 2015 at 5:36 PM, Mike Rappazzo <rappazzo@xxxxxxxxx> wrote: > On Wed, Sep 16, 2015 at 5:09 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >> On Mon, Sep 14, 2015 at 1:44 PM, Mike Rappazzo <rappazzo@xxxxxxxxx> wrote: >>> On Sat, Sep 12, 2015 at 11:19 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: >>>> The original code in branch.c, which this patch removes, did not need >>>> to make a special case of HEAD, so it's not immediately clear why this >>>> replacement code does so. This is the sort of issue which argues in >>>> favor of mutating the existing code (slowly) over the course of >>>> several patches into the final form, rather than having the final form >>>> come into existence out of thin air. When the changes are made >>>> incrementally, it is easier for reviewers to understand why such >>>> modifications are needed, which (hopefully) should lead to fewer >>>> questions such as this one. >>> >>> I reversed the the check here; it is intended to check if the symref >>> is _not_ the head, since the head >>> ref has already been parsed. This is used in notes.c to find >>> "NOTES_MERGE_REF". >> >> I'm probably being dense, but I still don't understand why the code >> now needs a special case for HEAD, whereas the original didn't. But, >> my denseness my be indicative of this change not being well-described >> (or described at all) by the commit message. Hopefully, when this is >> refactored into finer changes, the purpose will become clear. > > The special case for HEAD is because the HEAD is already included in > the worktree struct. This block is intended to save from re-parsing. > If you think the code would be easier to read, the HEAD check could be > removed, and the ref will just be parsed always. I'm unable to judge whether the code would be easier to read since I still don't understand the purpose of the special case. (But, as noted, it may just be that I'm being dense, though not intentionally.) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html