Re: [PATCH v8 1/2] [GSOC] commit: add --trailer option

From: ZheNing Hu <adlternative@gmail.com>
Date: Mar 17, 2021, 13:54 UTC
Message-ID: <CAOLTT8SFd=Zg01kR9rvTO6yh+tSS5VbcLBjAcB+d_DX_NY_HOw@mail.gmail.com>
In-Reply-To: <87wnu6dwxx.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 于2021年3月17日周三 下午4:08写道：

Show 11 quoted lines

> > Logically speaking, `interpret_trailers` should be dedicated to `commit`
> > or other sub-commands that require trailers.
> >
> > But I think that in the later stage, the parse_options of the `cmd_commit`
> > can keep the unrecognized options, and then these choices can be directly
> > passed to the `interpret_trailers` backend.
>
> We have this interaction with e.g. range-diff and "log", it's often
> surprising. You add an option to one command and it appears in the
> other.
>

All right, I'm wrong, I may have reference to an wrong experience of `difftool`-->`diff`.

Show 38 quoted lines

> >>    It seems to me to be a good idea to (at least for testing) convert
> >>    the --signoff trailer to your implementation. We have plenty of tests
> >>    for it, does migrating it over pass or fail those?
> >>
> > I don’t know how to migrating yet, it may take a long time.
> > Even I think I can leave it as #leftoverbit later.
>
> Sure, I mean (having looked at it) that at least for your own local
> testing it would make sense to change it (even if just search-replacing
> the --signoff in the test suite) to see if it behaves as you
> expect. I.e. does the --trailer behavior mirror --signoff?
>
> >>  * I also agree with Junio that we shouldn't have a --fixed-by or
> >>    whatever and wouldn't add --signoff today, but it seems very useful
> >>    to me to have a shortcut like:
> >>
> >>        --trailer "Signed-off-by"
> >>
> >>    I.e. omitting the value, or:
> >>
> >>       --trailer "Signed-off-by="
> >>
> >>    Or some other thing we deem sufficiently useful/sane
> >>    syntax/unambiguous.n
> >>
> >>    Then the value would be provided by fmt_name(WANT_COMMITTER_IDENT)
> >>    just as we do in append_signoff() now. I think a *very common* case
> >>    for this would be something like:
> >>
> >>        git commit --amend -v --trailer "Reviewed-by"
> >>
> >>    And it would be useful to help that along and not have to do:
> >>
> >>        git commit --amend -v --trailer "Reviewed-by=$(git config user.name) <$(git config user.email)>"
> >>
> >>    Or worse yet, manually typo your name/e-mail address, as I'm sure I
> >>    and many others will inevitably do when using this option...
> >>

Well, that's what I think here:

Now we can go through:

$ git -c trailer.signoff.key = "Signed-off-by" commit --trailer "signoff = commiter <email>"

to get a trailer: "Signed-off-by: commiter <email>", this means we
can't just do simple string
matching in `cmd_commit` to replace `--trailer="Signed-off-by"` or
`--trailer="Reviewed-by"` to
user's own identity, to replace the trailers which have omitting value
 we passed in, but I think
we can provide a new option to `commit` which can mandatory that
trailers with no value can be
 replaced with the identity of the user.

e.g.

$ git -c trailer.signoff.key = "Signed-off-by" commit --trailer
"signoff" --trailer "Helped-by" \
 --trailer "Helped-by = C <E>" --own_ident

will output like this:

Signed-off-by: $(git config user.name) <$(git config user.email)>
Signed-off-by: $(git config user.name) <$(git config user.email)>
Helped-by: $(git config user.name) <$(git config user.email)>
Helped-by: C <E>

I don't know if this idea is good, I will try to do it first.

Thanks.