Re: [RFC/PATCH 2/2] trailer: add examples to the documentation

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2014, 22:12 UTC
Message-ID: <xmqqbnvlnlwc.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140427201238.16880.7144.chriscool@tuxfamily.org>

Christian Couder <chriscool@tuxfamily.org> writes:

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  Documentation/git-interpret-trailers.txt | 98 +++++++++++++++++++++++++++++++-
>  1 file changed, 97 insertions(+), 1 deletion(-)

Very good to have these examples.  They seem to be clearly written.

Having said that, this part made me go "Huh" for a few reasons.

Show 20 quoted lines

> +* Configure a 'fix' trailer with a command to show the subject of a
> +  commit that is fixed, and show how it works:
> ++
> +------------
> +$ git config trailer.fix.key "Fixes #"
> +$ git config trailer.fix.ifExists "overwrite"
> +$ git config trailer.fix.command "git log -1 --oneline --format=\"%h (%s)\" --abbrev-commit --abbrev=14 \$ARG"
> +$ git interpret-trailers <<EOF
> +> subject
> +> 
> +> message
> +> 
> +> fix: HEAD~2
> +> EOF
> +subject
> +
> +message
> +
> +Fixes #fe3187489d69c4 (subject of fixed commit)
> +------------

 - It makes it appear that we have a convention to prefix "#" when
   talking about a commit object name, but that is not what we
   recommend to do.  Do some projects refer to commit object names
   that way?

 - It has been my impression that "#123456", when people talk about
   "Fixes #123456", refers to a bug ID in some tracking system, and
   the scenario the example sets up "show the subject that is fixed"
   would not help those who want to have such a linkage between bug
   ID and fix.

I however do not think of a good mechanised way to fill in the bug ID with script [*1*]. I suspect (but I am thinking aloud and am open to better alternatives) it would be better to change this example *not* to show use of trailer.*.command but have it just take the bug ID literally to make it a demonstration that the trailer does not have to end with "key: ". We can of course have your example to turn a commit object name to human readable one-liner as a separate one to illustrate how to use trailer.*.command, but I do not think it is a good idea to call it "Fixes", as that may invite confusions with a bug ID people would expect to see.

[Footnote]

*1* A workable project convention may be to document an existing bug
    in a test-suite and detect a commit that fixes the bug, e.g.

    - Document an expected-to-fail bug like so:

      test_expect_failure 'bug #123456' '
	... demonstrate breakage ...
      '

      when it is noticed to keep track of it.

    - A commit that fixes the bug will have a hunk:

      -test_expect_failure 'bug #123456' '
      +test_expect_success 'bug #123456' '

      and the trailer.fix.command can examine "git show" output of
      the proposed commit to detect that hunk and extract "bug #"
      from there.

    Of course that is not _the_ single best convention, and it would
    be too big for an example in this documentation.