| Message ID | 20190219170709.25463-1-newren@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] merge-options.txt: correct wording of --no-commit option | expand |
Elijah Newren <newren@gmail.com> writes: > +With --no-commit perform the merge and stop just before creating > +a merge commit, to give the user a chance to inspect and further > +tweak the merge result before committing. > ++ > +Note that fast-forward updates do not need to create a merge > +commit and therefore there is no way to stop those merges with > +--no-commit. Thus, if you want to ensure your branch is not > +changed or updated by the merge command, use --no-ff with > +--no-commit. While the above is an improvement (so I'll queue it on 'pu' not to lose sight of it), I find the use of "do not need to" above somewhat misleading. It solicits a reaction "ok, we know it does not need to, but it could prepare to create one to allow us to further muck with it, no?". IOW, a fast-forward by definition does not create a merge by itself, so there is nowhere to stop during a creation of a merge. So at least: s/do not need to/do not/ It also may be a good idea to consider detecting this case and be a bit more helpful, perhaps with end-user experience looking like... $ git checkout master^0 $ git merge --no-commit next Updating 0d0ac3826a..ee538a81fe Fast-forward ...diffstat follows here... hint: merge completed without creating a commit. hint: if you wanted to prepare for a manually tweaked merge, hint: do "git reset --keep ORIG_HEAD" followed by hint: "git merge --no-ff --no-commit next". or even $ git checkout master^0 $ git merge --no-commit next warning: defaulting to --no-ff, given a --no-commit request Automatic merge went well; stopped before committing as requested hint: if you'd rather have a fast-forward without creating a commit, hint: do "git reset --keep next" now. I do not have a strong preference among three (the third option being not doing anything), but if pressed, I'd say that the last one might be the most user-friendly, even though it feels a bit too magical and trying to be smarter than its own good. In any case, the hint for the "recovery" procedure needs to be carefully written.
On Tue, Feb 19, 2019 at 11:32 AM Junio C Hamano <gitster@pobox.com> wrote: > Elijah Newren <newren@gmail.com> writes: > > > +With --no-commit perform the merge and stop just before creating > > +a merge commit, to give the user a chance to inspect and further > > +tweak the merge result before committing. > > ++ > > +Note that fast-forward updates do not need to create a merge > > +commit and therefore there is no way to stop those merges with > > +--no-commit. Thus, if you want to ensure your branch is not > > +changed or updated by the merge command, use --no-ff with > > +--no-commit. > > While the above is an improvement (so I'll queue it on 'pu' not to > lose sight of it), I find the use of "do not need to" above somewhat > misleading. It solicits a reaction "ok, we know it does not need > to, but it could prepare to create one to allow us to further muck > with it, no?". > > IOW, a fast-forward by definition does not create a merge by itself, > so there is nowhere to stop during a creation of a merge. So at > least: > > s/do not need to/do not/ Yes, I agree that's a good change. I'll wait a few days for other feedback and resend with that and any other changes. > It also may be a good idea to consider detecting this case and be a > bit more helpful, perhaps with end-user experience looking like... > > $ git checkout master^0 > $ git merge --no-commit next > Updating 0d0ac3826a..ee538a81fe > Fast-forward > ...diffstat follows here... > hint: merge completed without creating a commit. > hint: if you wanted to prepare for a manually tweaked merge, > hint: do "git reset --keep ORIG_HEAD" followed by > hint: "git merge --no-ff --no-commit next". > > or even > > $ git checkout master^0 > $ git merge --no-commit next > warning: defaulting to --no-ff, given a --no-commit request > Automatic merge went well; stopped before committing as requested > hint: if you'd rather have a fast-forward without creating a commit, > hint: do "git reset --keep next" now. Good points. I thought of this last one before sending, though without pre- and post- warnings/hints; without such text it definitely seemed too magical and possibly leading to unexpected surprises in a different direction, so I dismissed it without further thought. But the warnings/hints help. > I do not have a strong preference among three (the third option > being not doing anything), but if pressed, I'd say that the last one > might be the most user-friendly, even though it feels a bit too > magical and trying to be smarter than its own good. I also lack a strong preference. Maybe mark it #leftoverbits for someone that does? > In any case, the hint for the "recovery" procedure needs to be > carefully written. Yes.
Elijah Newren <newren@gmail.com> writes: >> $ git checkout master^0 >> $ git merge --no-commit next >> warning: defaulting to --no-ff, given a --no-commit request >> Automatic merge went well; stopped before committing as requested >> hint: if you'd rather have a fast-forward without creating a commit, >> hint: do "git reset --keep next" now. > > Good points. I thought of this last one before sending, though > without pre- and post- warnings/hints; without such text it definitely > seemed too magical and possibly leading to unexpected surprises in a > different direction, so I dismissed it without further thought. But > the warnings/hints help. > >> I do not have a strong preference among three (the third option >> being not doing anything), but if pressed, I'd say that the last one >> might be the most user-friendly, even though it feels a bit too >> magical and trying to be smarter than its own good. > > I also lack a strong preference. Maybe mark it #leftoverbits for > someone that does? This definitely is outside the scope of the documentation update.
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index c2a263ba74..d1061b8cf7 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -3,9 +3,15 @@ Perform the merge and commit the result. This option can be used to override --no-commit. + -With --no-commit perform the merge but pretend the merge -failed and do not autocommit, to give the user a chance to -inspect and further tweak the merge result before committing. +With --no-commit perform the merge and stop just before creating +a merge commit, to give the user a chance to inspect and further +tweak the merge result before committing. ++ +Note that fast-forward updates do not need to create a merge +commit and therefore there is no way to stop those merges with +--no-commit. Thus, if you want to ensure your branch is not +changed or updated by the merge command, use --no-ff with +--no-commit. --edit:: -e::
The former wording implied that --no-commit would always cause the merge operation to "pause" and allow the user to make further changes and/or provide a special commit message for the merge commit. This is not the case for fast-forward merges, as there is no merge commit to create. Without a merge commit, there is no place where it makes sense to "stop the merge and allow the user to tweak changes"; doing that would require a full rebase of some sort. Since users may be unaware of whether their branches have diverged or not, modify the wording to correctly address fast-forward cases as well and suggest using --no-ff with --no-commit if the point is to ensure that the merge stops before completing. Reported-by: Ulrich Windl <Ulrich.Windl@rz.uni-regensburg.de> Signed-off-by: Elijah Newren <newren@gmail.com> --- Changes since v1: - Tweaked commit message Documentation/merge-options.txt | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)