From 7009d7d03d8e7f50d35a0ba9b3793663ae88bb45 Mon Sep 17 00:00:00 2001 From: Jeremy Stanley Date: Fri, 15 Jul 2022 17:24:11 +0000 Subject: [PATCH] Clarify that test rebases are not kept There has been a long-standing misconception that git-review pushes automatically rebased changes by default. It does not, but our documentation and context help have been less than clear on that point, contributing to this impression. Try to do a better job of explaining that the default rebasing performed by git-review is purely exploratory, and used only to notify users about possible merge conflicts with their target branch before pushing a change. Change-Id: I3c841af5ff9430a0de4d9dc9526dd3be6ab53ad2 --- doc/source/usage.rst | 3 ++- git-review.1 | 19 ++++++++++++++----- git_review/cmd.py | 6 ++++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/doc/source/usage.rst b/doc/source/usage.rst index 307b1e2e..abbdeeaa 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -30,7 +30,8 @@ If you want to submit a branch for review and then remove the local branch:: git review -f -If you want to skip the automatic "git rebase -i" step:: +If you want to be able to push a change which has a merge conflict with the +remote branch:: git review -R diff --git a/git-review.1 b/git-review.1 index c20e06c4..15a4bd83 100644 --- a/git-review.1 +++ b/git-review.1 @@ -131,12 +131,17 @@ If the master branch is different enough, the rebase can produce merge conflicts If that happens rebasing will be aborted and diff displayed for not\-rebased branches. You can also use .Ar \-\-no\-rebase ( Ar \-R ) -to always skip rebasing. +to always skip the test rebase. .It Fl f , Fl \-finish Close down the local branch and switch back to the target branch on successful submission. .It Fl F , Fl \-force\-rebase -Force a rebase before doing anything else, even if not otherwise needed. +Force a rebase before doing anything else, even if not otherwise needed. Unlike +the normal test rebase, this rebase will be kept. Use if you know you want to +push a commit automatically rebased onto the current state of the target +branch. This is discouraged if amending an existing change, since it creates +unnecessary additional differences from the previous revision and so makes +things harder for reviewers of your changes. .It Fl n , Fl \-dry\-run Don\(aqt actually perform any commands that have direct effects. Print them instead. @@ -171,10 +176,14 @@ one patch. .It Fl v , Fl \-verbose Turns on more verbose output. .It Fl R , Fl \-no\-rebase -Do not automatically perform a rebase before submitting the change to -Gerrit. +Don't test for possible merge conflicts with the target branch before pushing. +If the test rebase detects no merge conflicts then the rebase is undone and +your previous state is pushed. If merge conflicts are detected git\-review +exits with the rebase in progress allowing you to address it manually. By +default git\-review will never push the results of a rebase without your +explicit involvement. .Pp -When submitting a change for review, you will usually want it to be based on the tip of upstream branch in order to avoid possible conflicts. When amending a change and rebasing the new patchset, the Gerrit web interface will show a difference between the two patchsets which contains all commits in between. This may confuse many reviewers that would expect to see a much simpler difference. +Use the this option to skip the merge conflict test, allowing you to push merge conflicts. .Pp Also can be used for .Fl \-compare diff --git a/git_review/cmd.py b/git_review/cmd.py index f3df7289..e4b2dd5b 100644 --- a/git_review/cmd.py +++ b/git_review/cmd.py @@ -1540,10 +1540,12 @@ additional information: rebase_group = parser.add_mutually_exclusive_group() rebase_group.add_argument("-R", "--no-rebase", dest="rebase", action="store_false", - help="Don't rebase changes before submitting.") + help="Don't test for remote merge conflicts" + " before pushing.") rebase_group.add_argument("-F", "--force-rebase", dest="force_rebase", action="store_true", - help="Force rebase even when not needed.") + help="Force and push a rebase even when not" + " needed.") track_group = parser.add_mutually_exclusive_group() track_group.add_argument("--track", dest="track",