Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 5, 2017, 09:14 UTC
Message-ID: <xmqqzi966jn9.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqvajv9m2k.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

> I think the remaining points from my reviews are:
>
>  - It would be better to compute precomputable stuff when used atoms
>    are parsed, instead of running starts_with() in these functions;

After reading the original (before these three patches) code for populate_value(), I think reverting this mess back to how the thing was originally envisioned to be would be outside the scope of this addition. It was way too messy before your series than I expected.

So, unless you or your collaborator feel like helping us to clean up the codebase for better future, let's forget about that point, and leave the cleaning up for now.

For those who want to help with the #leftoverbits, the first step of cleaning up would be to introduce an enum field that sits next to used_atom[i].name, and have the if/else if/... cascades use that instead of these starts_with(name) to choose what value to grab from the given ref.