Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation

From: Alex Henrie <alexhenrie24@gmail.com>
Date: Jul 8, 2023, 18:56 UTC
Message-ID: <CAMMLpeSk7_2xn_atUoVeyFSHwE3TNDijSwDMo6PVbvf4XFUvtw@mail.gmail.com>
In-Reply-To: <82255166-49ac-3c10-1744-27d6d436822e@gmail.com>

On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

> This message would also benefit from adding explanation as to why this
> change is desirable.

OK, in v5 I'll add more explanation to the commit messages, including points brought up in this discussion.

Show 29 quoted lines

> >   static const char message_advice_pull_before_push[] =
> >       N_("Updates were rejected because the tip of your current branch is behind\n"
> > -        "its remote counterpart. Integrate the remote changes (e.g.\n"
> > -        "'git pull ...') before pushing again.\n"
> > +        "its remote counterpart. Use 'git pull' to integrate the remote changes\n"
>
> This is much clearer than "(e.g. 'git pull ...')"
>
> > +        "before pushing again, or use 'git push --force' to delete the remote\n"
> > +        "changes and replace them with your own.\n"
>
> I think it would be good to give a bit more context here as to when
> force pushing is a good idea. For example something like
>
>      If you have rebased the branch since you last integrated remote
>      changes then you can use
>      'git push --force-with-lease=<branch-ref> --force-if-includes' to
>      safely replace the remote branch.
>
>      If you have deleted and then recreated the branch since you last
>      integrated remote changes then you can use 'git push +<branch>' to
>      replace the remote. Note that if anyone else has pushed work to
>      this branch it will be deleted.
>
> It makes the advice longer  but the user get a specific suggestion for
> their current situation rather than a generic suggestion to delete the
> remote changes without discussing the implications. In this case we know
> that it was the current branch that was rejected and so should fill in
> the branch name in the advice as well.

Even if we could fill in <branch-ref> automatically, it's too much to ask the user to type out --force-with-lease=<branch-ref> --force-if-includes. Mentioning `git push --force` with a fat warning about how it only makes sense in a narrow (but common) case would be enough to make users aware of it while deterring them from abusing it. The advice already refers the user to the man page for more information, which includes a discussion of --force-with-lease and --force-if-includes as alternatives to plain --force.

Show 5 quoted lines

> My main issue with the changes in this series is that they seem to
> assume the user is (a) pushing a single branch and (b) they are the only
> person who works on that branch. That is a common but narrow case where
> force pushing is perfectly sensible but there are many other scenarios
> where suggesting "push --force" would not be a good idea.

The goal of the series is not to assume that the user's situation is that narrow but common case, but rather to not assume that the user's situation is not that case. The most important thing is to make the user aware that integration/reconciliation is not the only possible way forward.

Thanks for the feedback,

-Alex