All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org, J Wyman <jwyman@microsoft.com>
Subject: Re: [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name.
Date: Wed, 04 Oct 2017 20:41:55 +0900	[thread overview]
Message-ID: <xmqqvajv9m2k.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqq4lrfb7jk.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Wed, 04 Oct 2017 18:12:47 +0900")

Junio C Hamano <gitster@pobox.com> 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).

Thanks.

  reply	other threads:[~2017-10-04 11:42 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-02 13:56 [PATCH 0/3] for-each-ref: add :remote-ref and :remote-name specifiers Johannes Schindelin
2017-10-02 13:56 ` [PATCH 1/3] for-each-ref: let upstream/push optionally report the remote name Johannes Schindelin
2017-10-04  7:14   ` Junio C Hamano
2017-10-04  9:08     ` Junio C Hamano
2017-10-05 12:20     ` Johannes Schindelin
2017-10-02 13:57 ` [PATCH 2/3] for-each-ref: let upstream/push optionally report merge name Johannes Schindelin
2017-10-04  9:12   ` Junio C Hamano
2017-10-04 11:41     ` Junio C Hamano [this message]
2017-10-05  9:14       ` Junio C Hamano
2017-10-02 13:57 ` [PATCH 3/3] for-each-ref: test :remote-name and :remote-ref Johannes Schindelin
2017-10-05  1:54 ` [PATCH 0/3] for-each-ref: add :remote-ref and :remote-name specifiers Junio C Hamano
2017-10-05 12:19 ` [PATCH v2 0/3] for-each-ref: add :remoteref and :remotename specifiers Johannes Schindelin
2017-10-05 12:19   ` [PATCH v2 1/3] for-each-ref: let upstream/push optionally report the remote name Johannes Schindelin
2017-10-10  9:35     ` Junio C Hamano
2017-10-11  2:07     ` Junio C Hamano
2017-10-05 12:19   ` [PATCH v2 2/3] for-each-ref: let upstream/push optionally remote ref name Johannes Schindelin
2017-10-06  5:10     ` Junio C Hamano
2017-10-11  5:58       ` Junio C Hamano
2017-10-12 19:13         ` Johannes Schindelin
2017-10-13  0:30           ` Junio C Hamano
2017-10-15 16:02             ` Johannes Schindelin
2017-10-13 16:39     ` Kevin Daudt
2017-10-14  2:08       ` Junio C Hamano
2017-10-15 16:05         ` Johannes Schindelin
2017-10-16  1:50           ` Junio C Hamano
2017-10-16 11:39             ` Johannes Schindelin
2017-10-05 12:19   ` [PATCH v2 3/3] for-each-ref: test :remotename and :remoteref Johannes Schindelin
2017-11-07 16:30   ` [PATCH v3 0/3] for-each-ref: add :remoteref and :remotename specifiers Johannes Schindelin
2017-11-07 16:30     ` [PATCH v3 1/3] for-each-ref: let upstream/push optionally report the remote name Johannes Schindelin
2017-11-07 16:31     ` [PATCH v3 2/3] for-each-ref: let upstream/push report the remote ref name Johannes Schindelin
2017-11-07 16:31     ` [PATCH v3 3/3] for-each-ref: test :remotename and :remoteref Johannes Schindelin
2017-11-08  1:36     ` [PATCH v3 0/3] for-each-ref: add :remoteref and :remotename specifiers Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqvajv9m2k.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=jwyman@microsoft.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.