Re: [PATCH v13] [GSOC] commit: add --trailer option

Christian Couder [off-list ref] 于2021年3月22日周一 下午3:43写道：

On Mon, Mar 22, 2021 at 5:24 AM ZheNing Hu via GitGitGadget
[off-list ref] wrote:

quoted

From: ZheNing Hu <redacted>

Historically, Git has supported the 'Signed-off-by' commit trailer
using the '--signoff' and the '-s' option from the command line.
But users may need to provide other trailer information from the
command line such as "Helped-by", "Reported-by", "Mentored-by",

Now implement a new `--trailer <token>[(=|:)<value>]` option to pass
other trailers to `interpret-trailers` and insert them into commit
messages.

Signed-off-by: ZheNing Hu <redacted>
Reported-by:

Why is there this "Reported-by:" trailer with an empty value? If you
are looking to add trailers to this commit message, you might want to
add them before your "Signed-off-by".

Sorry, It was purely a small accident during testing and I didn't notice it.

quoted

---
    [GSOC] commit: add --trailer option

    Now maintainers or developers can also use commit
    --trailer="Signed-off-by:commiter<email>" from the command line to
    provide trailers to commit messages. This solution may be more
    generalized than v1.

It's not a big deal as this is not going into the commit message, but
at this point (v13) you might want to tell explicitly what solution v1
implemented, instead of referring to it.

Ok.

quoted

     @@ t/t7502-commit-porcelain.sh: test_expect_success 'sign off' '>       + sed -e "1,/^\$/d" commit.msg >actual &&
      + test_cmp expected actual
      +'
     ++
     ++test_expect_success 'commit --trailer with -c and command' '
     ++ trailer_commit_base &&
     ++ cat >expected <<-\EOF &&
     ++ hello
     ++
     ++ Signed-off-by: C O Mitter [off-list ref]
     ++ Signed-off-by: C1 E1
     ++ Helped-by: C2 E2
     ++ Mentored-by: C4 E4
     ++ Reported-by: A U Thor [off-list ref]
     ++ EOF
     ++ git -c trailer.report.key="Reported-by: " \
     ++         -c trailer.report.ifexists="replace" \
     ++         -c trailer.report.command="git log --author=\"\$ARG\" -1 \
     ++         --format=\"format:%aN <%aE>\"" \
     ++         commit --trailer "report = author" --amend &&
     ++ git cat-file commit HEAD >commit.msg &&
     ++ sed -e "1,/^\$/d" commit.msg >actual &&
     ++ test_cmp expected actual
     ++'

Nice that you have added such a test!

Thanks.

But at the same time I have two little doubt.

1.
If we have your config:

$ git config trailer.sign.key "Signed-off-by: "
$ git config trailer.sign.ifexists replace
$ git config trailer.sign.command "git log --author='\$ARG' -1
--format='format:%aN <%aE>'"

Then I touch a test.c and use:

$ git interpret-trailers --in-place test.c

without `--trailer`, See what is happen:

It seem like your local repo last commit "name <email>" pair
have been record in `test.c`.

Could this be considered a bug?

2.
`git interpret-trailers --in-place`  seem like work on git top-dir,
If I am in a sub-dir `b` and I want to change a file such as `d.c`,
then I must use `git interpret-trailers --in-place b/d.c` to add some
trailers.

I think the original intention of `--in-place` is to modify a file similar to
"$COMMIT_MSG_FILE", so make it run at top-dir, but this is not reflected
in the git documentation. This at least confuses people who use this
option for the first time. Is it worth modifying? Or is there something
wrong with the design of `--in-place`?

--
ZheNing Hu