"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: draft: keeping authorship of diffs in commits
To:
Omar Polo <op@omarpolo.com>
Cc:
gameoftrees@openbsd.org
Date:
Tue, 19 Jul 2022 15:52:52 +0200

Download raw body.

Thread
On Tue, Jul 19, 2022 at 03:31:44PM +0200, Omar Polo wrote:
> rebased again after recent change to valid_author; now it's a bit
> smaller.
> 
> (i'm still trying to come up with a description that i like for the
> manpage)

> @@ -1541,6 +1541,13 @@ The options for
>  .Cm got commit
>  are as follows:
>  .Bl -tag -width Ds
> +.It Fl A Ar author
> +Specify the
> +.Ar author
> +of the commit with the same format of
> +.Ev GOT_AUTHOR .
> +The user doing the commit is then acknowledged as the
> +.Dq committer .

How about this?

.It Fl A Ar author
Set author information in the newly created commit to
.Ar author .
This is useful when committing changes which were written by someone else.
The
.Ar author
argument must use the same format as the
.Ev GOT_AUTHOR
environment variable.
.Pp
In addition to storing author information, the newly created commit object
will retain
.Dq committer
information which is obtained, as usual, from the
.Ev GOT_AUTHOR
environment variable, or
.Xr got.conf 5 ,
or Git configuration settings.

> @@ -8489,8 +8490,13 @@ cmd_commit(int argc, char *argv[])
>  	TAILQ_INIT(&paths);
>  	cl_arg.logmsg_path = NULL;
>  
> -	while ((ch = getopt(argc, argv, "F:m:NS")) != -1) {
> +	while ((ch = getopt(argc, argv, "A:F:m:NS")) != -1) {
>  		switch (ch) {
> +		case 'A':
> +			author = optarg;
> +			if ((error = valid_author(author)) != NULL)
> +				return error;

Please avoid assigments inside if-statements.
I recommend this idiom instead:

			error = valid_author(author);
			if (error)
				return errror;

> @@ -8604,7 +8613,7 @@ cmd_commit(int argc, char *argv[])
>  		cl_arg.branch_name += 11;
>  	}
>  	cl_arg.repo_path = got_repo_get_path(repo);
> -	error = got_worktree_commit(&id, worktree, &paths, author, NULL,
> +	error = got_worktree_commit(&id, worktree, &paths, author, committer,
>  	    allow_bad_symlinks, collect_commit_logmsg, &cl_arg,
>  	    print_status, NULL, repo);
>  	if (error) {

What happens when author and committer strings have the same content?
Should we set committer to NULL in this case?