Re: [PATCH v2] [GSOC] commit: add trailer command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Mar 12, 2021 at 4:59 PM ZheNing Hu via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: ZheNing Hu <adlternative@xxxxxxxxx>
>
> 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 richer trailer information from the
> command line such as "Helped-by", "Reported-by", "Mentored-by",

Nit: not sure that "richer" is the proper word here. I would just use
"other" instead.

> Now use `--trailer <token>[(=|:)<value>]` pass the trailers to
> `interpret-trailers` and generate trailers in commit messages.

The subject says "add trailer command" while here you say "use". So
which one is it? Does "--trailer" already exist, and we are just going
to use it? Or will this patch series actually "add" it?

Looking at the existing options and the code of this patch series, the
patch series actually adds the "--trailer" (not "trailer") option, so
"add" or "implement" would be clearer than "use".

So maybe something like the following might be better:

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

Also something like "--trailer" is usually called an option (or
sometimes a flag), not a command (especially not when the word is not
a verb, and when the new feature isn't a new exclusive mode of
operation). So something like "commit: add --trailer option" might be
a better subject.

> Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx>
> ---
>     [GSOC] commit: provides multiple signatures from command line

It looks like this is using the subject of a patch that previously
attempted to add features with a similar purpose. I don't think you
need to put it there, or if you want to refer to it, I think it might
be better to be a bit more explicit, for example like:

"This patch replaces my previous attempt to provide similar features
in a patch called: [GSOC] commit: provides multiple signatures from
command line."

>     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.

Ok, I agree that it's a good idea to have a good generic solution
first, before having specialized options for specific trailers.

> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-901%2Fadlternative%2Fcommit-with-multiple-signatures-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-901/adlternative/commit-with-multiple-signatures-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/901
>
> Range-diff vs v1:

If this patch series has very few code and commit messages in common
with a previous attempt at implementing similar features, it might be
better to make it a new patch series rather than a v2. This could
avoid sending range-diffs that are mostly useless.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux