Re: [PATCH v3 2/2] push: advise about force-pushing as an alternative to reconciliation
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jul 11, 2023, 18:33 UTC
- Message-ID
- <3479e947-76ce-2eb6-8ae0-5360311c5967@gmail.com>
- In-Reply-To
- <CAMMLpeSk7_2xn_atUoVeyFSHwE3TNDijSwDMo6PVbvf4XFUvtw@mail.gmail.com>
Hi Alex
On 08/07/2023 19:56, Alex Henrie wrote:
Show 26 quoted lines
> On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@gmail.com> wrote: >>> + "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.
Can't they just copy and paste the command from the advice message? Even if the user does not copy and paste it is not that hard to type it out with the benefit of the shell's tab completion. You're basically saying this combination of options is unusable in practice because it is too much effort to type them. We could look to see if we can make it less unwieldy by changing push to allow --force-if-includes=ref imply --force-with-lease for instance.
> 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.
Having a warning in the advice message would definitely help
> 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.
It is good to mention the man page in the advice but we shouldn't assume that users will actually go and read it before running the suggested command.
Show 11 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 clarifying, that is the sort of thing that should be in the commit message.
Best Wishes
Phillip
> Thanks for the feedback, > > -Alex