Re: What's cooking in git.git (Nov 2016, #05; Wed, 23)

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 29, 2016, 04:43 UTC
Message-ID: <CAGf8dg+=Neozuacsp21Neg=wp30k84bvfnBYs-bOoyTjMGvzVA@mail.gmail.com>
In-Reply-To: <xmqqpolfgndq.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 28, 2016 at 4:06 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 32 quoted lines

> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> On 11/23/2016 03:21 PM, Junio C Hamano wrote:
>>> * jt/use-trailer-api-in-commands (2016-11-02) 6 commits
>>>  - sequencer: use trailer's trailer layout
>>>  - trailer: have function to describe trailer layout
>>>  - trailer: avoid unnecessary splitting on lines
>>>  - commit: make ignore_non_trailer take buf/len
>>>  - SQUASH???
>>>  - trailer: be stricter in parsing separators
>>>
>>>  Commands that operate on a log message and add lines to the trailer
>>>  blocks, such as "format-patch -s", "cherry-pick (-x|-s)", and
>>>  "commit -s", have been taught to use the logic of and share the
>>>  code with "git interpret-trailer".
>>>
>>>  What's the doneness of this topic?
>>
>> Stefan Beller mentioned [1] that this seemed OK to him from a cursory
>> read. Do I need to look for another reviewer (or a more thorough
>> review)?
>
> I gave it a cursory review when it was queued, too, so another
> cursory read does not help very much ;)  If I recall correctly, I
> got an impression that it was reasonably well done.
>
> I haven't had a chance to look at the series again to see if the
> SQUASH is just the simple matter of squashing it into the one
> previous, which is the main reason why I haven't decided if it is
> ready to be in 'next'.
>
> Thanks.

Sorry, I'm not sure what you mean by this. The SQUASH is an update in documentation (for a C function) (reproduced below [1]) which can be squashed cleanly (just to confirm, I tried it using "git rebase -i"). It can also be rebased cleanly onto master or next (at least, as of now).

If you are asking about whether I think the contents of your SQUASH commit is reasonable, I think that it is.

[1]
$ git show 7e7587d
commit 7e7587d563ddb540875075e5a84359a8a96a5188
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed Nov 2 19:28:53 2016 -0700

    SQUASH???

diff --git a/trailer.c b/trailer.c
index dc525e3..eefe91d 100644
--- a/trailer.c
+++ b/trailer.c
@@ -565,7 +565,9 @@ static int token_matches_item(const char *tok,
struct arg_item *item, int tok_le
 /*
  * If the given line is of the form
  * "<token><optional whitespace><separator>..." or "<separator>...", return the
- * location of the separator. Otherwise, return -1.
+ * location of the separator. Otherwise, return -1.  The optional whitespace
+ * is allowed there primarily to allow things like "Bug #43" where <token> is
+ * "Bug" and <separator> is "#".
  *
  * The separator-starts-line case (in which this function returns 0) is
  * distinguished from the non-well-formed-line case (in which this function