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

From: Shourya Shukla <periperidip@gmail.com>
Date: Mar 17, 2021, 05:26 UTC
Message-ID: <20210317052641.GA8212@konoha>
In-Reply-To: <pull.901.v9.git.1615891183320.gitgitgadget@gmail.com>

On 16/03 10:39, ZheNing Hu via GitGitGadget wrote:

Hey ZheNing!

Show 12 quoted lines

> 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 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 <adlternative@gmail.com>

I have been away for a while and directly seeing a V9 of this patch feels great! Its good that you have worked upon the patch. The above approach seems good to me!

Show 8 quoted lines

>  	/*
>  	 * Reject an attempt to record a non-merge empty commit without
>  	 * explicit --allow-empty. In the cherry-pick case, it may be
> @@ -1507,6 +1528,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  		OPT_STRING(0, "fixup", &fixup_message, N_("commit"), N_("use autosquash formatted message to fixup specified commit")),
>  		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
>  		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
> +		OPT_CALLBACK_F(0, "trailer", NULL, N_("trailer"), N_("trailer(s) to add"), PARSE_OPT_NONEG, opt_pass_trailer),

I feel that a better option description could be offered? Maybe something like: 'add custom trailer(s)'.

>  		OPT_BOOL('s', "signoff", &signoff, N_("add a Signed-off-by trailer")),
>  		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
>  		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),

I have not yet gone through the the V2-V8s but I have a comment not associated with the contents of the patch. I feel that you should wait a little before posting a new version of the patch. I see that V4-V8 are put up in almost 3 hour gaps. This isn't technically wrong or prohibited by the communication rules of the List but I feel that posting a patch in such short intervals makes it hard to review and unnecessarily increases the versions of the patch. The reviewer lags behind the patch series in fact.

What you could do instead is post one patch per day instead of 3-4 in one single day so that your patches get thorough reviews. This way, you won't create 3-4 new versions of the patch containing not-so-many significant changes. You get me?

Also, in your reply on the V1 here: https://lore.kernel.org/git/CAOLTT8SpAOj51jqYUYqYwXaVKSn1fANvetauaG0z4etiBMzVEw@mail.gmail.com/

I read:

> It's exactly what you said.
> My lack of English sometimes limits my expression.

It is okay please do not worry. Neither do we have English as our first language nor have we ever communicated this much with an English speaking audience online. I struggled initially too especially with many American terms used here. You will get the hang of it soon.

Keep contributing!

Regards, Shourya Shukla