Thread (55 messages) 55 messages, 7 authors, 2021-03-23

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

flat view

From: ZheNing Hu <hidden>
Date: 2021-03-17 13:55:58

Ævar Arnfjörð Bjarmason [off-list ref] 于2021年3月17日周三 下午4:08写道:
quoted
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`.
quoted
quoted
   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?
quoted
quoted
 * 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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help