Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.
- Date: Wed, 04 Oct 2017 20:41:55 +0900
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.
Junio C Hamano <gitster@xxxxxxxxx> writes:
>> + if (explicit)
>> + *s = xstrdup(merge);
>> + else
>> + *s = "";
> Here is the same "who are we trying to help---users who want to know
> where their push goes, or users who are debugging where the push
> destination is defined?" question. I do not have a strong opinion
> either way, but I think giving the end result with fallback (i.e.
> not nullifying when the result is not explicit) may be easier to use
> by "push" users.
Now after thinking about it a bit more, I actually have a moderate
preference to doing it the way your patches do. With programatic
%(if)%(else)%(end) support we acquired recently, the fallback can be
coded in the --format=... language if the user wanted to using the
"internal fallbacks, explicit==0, are ignored" behaviour that are
implemented by these two patches. The reverse is not true.
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;
- We want to make sure there is no existing multi-word modifiers
(in which case we can safely declare "multi-word" is the way we
spell them from now on). Or if there are existing ones, they
already spell themselves as "multi-word".
I have nothing against "remote-name"; I just want to make sure we
are not making things inconsistent. If there are only few (but
non zero number of) multi-word modifiers that are not spelled
"multi-word", as long as they are only few and their spelling are
inferiour (e.g. concatenatedwords is much worse than
concatenated-words), we could still declare "multi-word" is the
right way to spell them going forward, declare that we will give
them synonyms and deprecate the bad spelling out over time, and
leave that plan as #leftover bits thing (i.e. not doing the
deprecation of these other modifiers as part of this series.
The only thing I want to happen in the scope of _this_ series, as
due diligence, is to make sure we are happy with "multi-word",
and also to know if we need a follow-up work (just yes/no,
possibly with plans, but no actual work yet).