git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Alex Henrie <alexhenrie24@gmail.com>,
	git@vger.kernel.org, git@matthieu-moy.fr, christiwald@gmail.com,
	john@keeping.me.uk, philipoakley@iee.email,
	phillip.wood@dunelm.org.uk
Subject: Re: [PATCH v5 0/3] don't imply that integration is always required before pushing
Date: Thu, 13 Jul 2023 10:51:09 +0100	[thread overview]
Message-ID: <919d1ba8-bb8b-a77b-cef3-db14f168ed4a@gmail.com> (raw)
In-Reply-To: <20230713044128.3771818-1-alexhenrie24@gmail.com>

Hi Alex

On 13/07/2023 05:41, Alex Henrie wrote:
> Many times now, I have seen novices do the following:
> 
> 1. Start work on their own personal topic branch
> 2. Push the branch to origin
> 3. Rebase the branch onto origin/master
> 4. Try to push again, but Git says they need to pull
> 5. Pull and make a mess trying to reconcile the older topic branch with
>     the rebased topic branch
> 
> Help avoid this mistake by giving somewhat more general advice that does
> not assume that the user always wants to do reconciliation.
> 
> Changes from v4:
> - Don't show divergent branch advice in the middle of `git commit`
> - Soften the advice, but don't specifically mention force-pushing

All three patches look fine to me, they are definitely an improvement on 
the current advice.

Thanks for working on this,

Phillip

> Alex Henrie (3):
>    wt-status: don't show divergence advice when committing
>    remote: don't imply that integration is always required before pushing
>    push: don't imply that integration is always required before pushing
> 
>   builtin/checkout.c |  2 +-
>   builtin/push.c     | 24 +++++++++++------------
>   remote.c           |  8 +++++---
>   remote.h           |  3 ++-
>   t/t7508-status.sh  | 48 ++++++++++++++++++++++------------------------
>   wt-status.c        |  3 ++-
>   6 files changed, 45 insertions(+), 43 deletions(-)
> 
> Range-diff against v4:
> 1:  9626721c13 < -:  ---------- remote: advise about force-pushing as an alternative to reconciliation
> -:  ---------- > 1:  e84989c4a6 wt-status: don't show divergence advice when committing
> -:  ---------- > 2:  9bb643df7e remote: don't imply that integration is always required before pushing
> 2:  209e86588a ! 3:  5ff9ecb51b push: advise about force-pushing as an alternative to reconciliation
>      @@ Metadata
>       Author: Alex Henrie <alexhenrie24@gmail.com>
>       
>        ## Commit message ##
>      -    push: advise about force-pushing as an alternative to reconciliation
>      +    push: don't imply that integration is always required before pushing
>       
>      -    Also, don't put `git pull` in an awkward parenthetical, because
>      -    `git pull` can always be used to reconcile branches and is the normal
>      -    way to do so.
>      +    In a narrow but common case, the user is the only author of a branch and
>      +    doesn't mind overwriting the corresponding branch on the remote. This
>      +    workflow is especially common on GitHub, GitLab, and Gerrit, which keep
>      +    a permanent record of every version of a branch that is pushed while a
>      +    pull request is open for that branch. On those platforms, force-pushing
>      +    is encouraged and is analogous to emailing a new version of a patchset.
>      +
>      +    When giving advice about divergent branches, tell the user about
>      +    `git pull`, but don't unconditionally instruct the user to do it. A less
>      +    prescriptive message will help prevent users from thinking that they are
>      +    required to create an integrated history instead of simply replacing the
>      +    previous history. Also, don't put `git pull` in an awkward
>      +    parenthetical, because `git pull` can always be used to reconcile
>      +    branches and is the normal way to do so.
>      +
>      +    Due to the difficulty of knowing which command for force-pushing is best
>      +    suited to the user's situation, no specific advice is given about
>      +    force-pushing. Instead, the user is directed to the Git documentation to
>      +    read about possible ways forward that do not involve integration.
>       
>           Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
>       
>      @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
>        	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"
>      -+	   "before pushing again, or use 'git push --force' to delete the remote\n"
>      -+	   "changes and replace them with your own.\n"
>      ++	   "its remote counterpart. If you want to integrate the remote changes,\n"
>      ++	   "use 'git pull' before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_checkout_pull_push[] =
>        	N_("Updates were rejected because a pushed branch tip is behind its remote\n"
>       -	   "counterpart. Check out this branch and integrate the remote changes\n"
>       -	   "(e.g. 'git pull ...') before pushing again.\n"
>      -+	   "counterpart. Check out this branch and use 'git pull' to integrate the\n"
>      -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
>      -+	   "the remote changes and replace them with your own.\n"
>      ++	   "counterpart. If you want to integrate the remote changes, use 'git pull'\n"
>      ++	   "before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_ref_fetch_first[] =
>      @@ builtin/push.c: static void setup_default_push_refspecs(int *flags, struct remot
>       -	   "(e.g., 'git pull ...') before pushing again.\n"
>       +	N_("Updates were rejected because the remote contains work that you do not\n"
>       +	   "have locally. This is usually caused by another repository pushing to\n"
>      -+	   "the same ref. Use 'git pull' to integrate the remote changes before\n"
>      -+	   "pushing again, or use 'git push --force' to delete the remote changes\n"
>      -+	   "and replace them with your own.\n"
>      ++	   "the same ref. If you want to integrate the remote changes, use\n"
>      ++	   "'git pull' before pushing again.\n"
>        	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static const char message_advice_ref_already_exists[] =
>      @@ builtin/push.c: static const char message_advice_ref_needs_force[] =
>       -	   "to integrate those changes locally (e.g., 'git pull ...')\n"
>       -	   "before forcing an update.\n");
>       +	N_("Updates were rejected because the tip of the remote-tracking branch has\n"
>      -+	   "been updated since the last checkout. Use 'git pull' to integrate the\n"
>      -+	   "remote changes before pushing again, or use 'git push --force' to delete\n"
>      -+	   "the remote changes and replace them with your own.\n");
>      ++	   "been updated since the last checkout. If you want to integrate the\n"
>      ++	   "remote changes, use 'git pull' before pushing again.\n"
>      ++	   "See the 'Note about fast-forwards' in 'git push --help' for details.");
>        
>        static void advise_pull_before_push(void)
>        {


  parent reply	other threads:[~2023-07-13  9:51 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-02 20:08 [PATCH 0/2] advise about force-pushing as an alternative to reconciliation Alex Henrie
2023-07-02 20:08 ` [PATCH 1/2] remote: " Alex Henrie
2023-07-02 20:08 ` [PATCH 2/2] push: " Alex Henrie
2023-07-03 15:33 ` [PATCH 0/2] " Phillip Wood
2023-07-03 16:26   ` Alex Henrie
2023-07-04 21:44   ` Junio C Hamano
2023-07-04 22:24     ` Alex Henrie
2023-07-05  5:30       ` Junio C Hamano
2023-07-06  2:32         ` Alex Henrie
2023-07-04 19:47 ` [PATCH v2 " Alex Henrie
2023-07-04 19:47   ` [PATCH v2 1/2] remote: " Alex Henrie
2023-07-04 21:51     ` Junio C Hamano
2023-07-04 22:41       ` Alex Henrie
2023-07-04 19:47   ` [PATCH v2 2/2] push: " Alex Henrie
2023-07-06  4:01   ` [PATCH v3 0/2] " Alex Henrie
2023-07-06  4:01     ` [PATCH v3 1/2] remote: " Alex Henrie
2023-07-06 20:25       ` Junio C Hamano
2023-07-06 20:40         ` Junio C Hamano
2023-07-06 23:23           ` Alex Henrie
2023-07-07 17:35             ` Junio C Hamano
2023-07-07 17:52             ` Junio C Hamano
2023-07-08 18:55               ` Alex Henrie
2023-07-09  1:38                 ` Junio C Hamano
2023-07-10  4:44                   ` Alex Henrie
2023-07-11  0:55                     ` Junio C Hamano
2023-07-12  4:47                       ` Alex Henrie
2023-07-12 15:18                         ` Junio C Hamano
2023-07-13  4:09                           ` Alex Henrie
2023-07-07  8:48       ` Phillip Wood
2023-07-06  4:01     ` [PATCH v3 2/2] push: " Alex Henrie
2023-07-07  8:49       ` Phillip Wood
2023-07-07 18:44         ` Junio C Hamano
2023-07-08 18:56         ` Alex Henrie
2023-07-11 18:33           ` Phillip Wood
2023-07-12  4:47             ` Alex Henrie
2023-07-12  4:55               ` Alex Henrie
2023-07-07  5:42     ` [PATCH v4 0/2] " Alex Henrie
2023-07-07  5:42       ` [PATCH v4 1/2] remote: " Alex Henrie
2023-07-07  5:42       ` [PATCH v4 2/2] push: " Alex Henrie
2023-07-13  4:41       ` [PATCH v5 0/3] don't imply that integration is always required before pushing Alex Henrie
2023-07-13  4:41         ` [PATCH v5 1/3] wt-status: don't show divergence advice when committing Alex Henrie
2023-07-13  4:41         ` [PATCH v5 2/3] remote: don't imply that integration is always required before pushing Alex Henrie
2023-07-13  4:41         ` [PATCH v5 3/3] push: " Alex Henrie
2023-07-13  9:51         ` Phillip Wood [this message]
2023-07-13 16:15           ` [PATCH v5 0/3] " Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=919d1ba8-bb8b-a77b-cef3-db14f168ed4a@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=alexhenrie24@gmail.com \
    --cc=christiwald@gmail.com \
    --cc=git@matthieu-moy.fr \
    --cc=git@vger.kernel.org \
    --cc=john@keeping.me.uk \
    --cc=philipoakley@iee.email \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).