Re: [PATCH v5 0/3] don't imply that integration is always required before pushing
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jul 13, 2023, 09:51 UTC
- Message-ID
- <919d1ba8-bb8b-a77b-cef3-db14f168ed4a@gmail.com>
- In-Reply-To
- <20230713044128.3771818-1-alexhenrie24@gmail.com>
Hi Alex
On 13/07/2023 05:41, Alex Henrie wrote:
Show 15 quoted lines
> 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
Show 98 quoted lines
> 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)
> {