From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got backout/cherrypick auto commit
To: Mark Jamsek <mark@jamsek.com>
Cc: Game of Trees <gameoftrees@openbsd.org>
Date: Thu, 19 Jan 2023 12:23:47 +0100

On Thu, Jan 19, 2023 at 07:05:44PM +1100, Mark Jamsek wrote:
> As per the subject and todo item, the below diff introduces the -a flag
> to both the backout and cherrypick commands.
> 
> I was glad to see this on the todo list because I'll use 'got cy -a'
> a lot! I often run log or tog to copypasta the log message before
> running `got cy branch && got ci -m msg` so I was keen to see this
> implemented.
> 
> I'm really looking for feedback though as I'm not sure if the annotated
> log message and base-commit bump stsp mentioned is adequate. Thanks!

Your code is not checking pre-conditions. This feature should require
a fully up-to-date and clean work tree to reduce the likelyhood of
problems during the merge and avoid unrelated changes getting mixed in.

> +	/*
> +	 * XXX Don't think we should error here as 'got {backout,cherrypick}'
> +	 * without -a doesn't error on these upa status codes.
> +	 */
> +	if (upa->conflicts || upa->missing || upa->not_deleted ||
> +	    upa->obstructed || upa->unversioned ||
> +	    upa->not_updated /* XXX shouldn't happen in cherrypick/backout? */)
> +		return NULL;

Erroring out here would be better because the user was asking for
an auto-commit an won't get it. I suppose the merge conflict stats
have already been printed at this point, so a generic error message
such as "aborting auto-commit due to unclean merge" should suffice.