Hi Alex
On 08/07/2023 19:56, Alex Henrie wrote:
On Fri, Jul 7, 2023 at 2:49 AM Phillip Wood <phillip.wood123@xxxxxxxxx> 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.
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
