Re: [PATCH v2] [GSOC] commit: add trailer command
- From
ZheNing Hu <adlternative@gmail.com>
- Date
- Mar 14, 2021, 07:09 UTC
- Message-ID
- <CAOLTT8SVW1Yy9emwpdxX6sw5cwDFV6fkJNugRtASzA9=vOrBbQ@mail.gmail.com>
- In-Reply-To
- <CAP8UFD2_SSza-zsoqS_ugQBryiTvm0PqzrJDAuriT2jmjHM-uA@mail.gmail.com>
Christian Couder <christian.couder@gmail.com> 于2021年3月14日周日 下午12:19写道:
Show 14 quoted lines
> > On Fri, Mar 12, 2021 at 4:59 PM ZheNing Hu via GitGitGadget > <gitgitgadget@gmail.com> wrote: > > > > From: ZheNing Hu <adlternative@gmail.com> > > > > 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. >
OK.
Show 11 quoted lines
> > 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". >
You're right. "add" will be more accurate in this situation.
Show 25 quoted lines
> 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@gmail.com> > > --- > > [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." >
I may have thought that the effect of the two patch was closer so did not change it.
Show 8 quoted lines
> > 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. >
Thanks. :)
Show 10 quoted lines
> > 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.
Thank you for these pertinent suggestions. I will pay more attention.