On Mon, Nov 28, 2016 at 4:06 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Jonathan Tan <jonathantanmy@xxxxxxxxxx> 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@xxxxxxxxx>
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