From: Stefan Sperling <stsp@stsp.name>
Subject: Re: got patch stats about conflicts and rejects
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Thu, 11 May 2023 09:53:09 +0200

On Thu, May 11, 2023 at 08:59:13AM +0200, Omar Polo wrote:
> On 2023/05/11 07:34:26 +0200, Stefan Sperling <stsp@stsp.name> wrote:
> > When 'got patch' sees merge conflicts it prints a final message saying
> > the patch failed to apply. This behaviour seemed somewhat unclear to me
> > in this particular case:
> > 
> > $ got patch < umb3.diff
> > G  Makefile
> > C  commands.c
> > G  conf.c
> > G  ctl.c
> > G  externs.h
> > G  if.c
> > G  main.c
> > C  ppp.c
> > G  sqlite3.c
> > A  umb.c
> > got: patch failed to apply
> > $
> > 
> > This left me wondering where the problem was. Other commands treat
> > conflicted files as an expected outcome rather than an error.
> 
> Admittedly I haven't noticed this difference.  I considered them a
> failure because the presence of a conflict means that we couldn't do
> what we were asked to.

Not always a failure. It will depend on the specific conflict whether
there is a real problem and only a human can judge this.

Applying patches that are based on out of date files can easily result
in conflicts that are trivial to resolve manually.

> > +struct got_patch_progress_arg {
> > +	int did_something;
> 
> I fail to see the usefulness of `did_something'.  Do you have some
> further diffs that would make use of it, or is it just to keep the
> same "interface" with the other progress_arg struct?

Generally the progress callbacks use this to prevent messages from
being printed when the callback was never called for some reason.

Of course since we don't print anything when values are non-zero
the flag is redundant in this particular case.
I don't have planned changes that would make this really necessary
for got patch but I suppose it won't hurt to stick to this idiom.
It might prevent surprises in case we change things later.