On Mon, Sep 14, 2015 at 1:44 PM, Mike Rappazzo <rappa...@gmail.com> wrote:
> On Sat, Sep 12, 2015 at 11:19 PM, Eric Sunshine <sunsh...@sunshineco.com>
> wrote:
>> On Fri, Sep 4, 2015 at 5:39 PM, Michael Rappazzo <rappa...@gmail.com> wrote:
>>> + while (!matched && worktree_list) {
>>> + if (strcmp("HEAD", symref)) {
>>> + strbuf_reset(&path);
>>> + strbuf_reset(&sb);
>>> + strbuf_addf(&path, "%s/%s",
>>> worktree_list->worktree->git_dir, symref);
>>> +
>>> + if (_parse_ref(path.buf, &sb, NULL)) {
>>> + continue;
>>> + }
>>> +
>>> + if (!strcmp(sb.buf, target))
>>> + matched = worktree_list->worktree;
>>
>> 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.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
