| Message ID | pull.1059.v2.git.git.1628054935.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Documentation updates: merge-strategies | expand |
"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > Changes since v1: > > * Multiple tweaks suggested by Eric, Dscho, and Junio > * Removed patch 7 explaining no-renames since that probably belongs in git > diff --no-renames instead, and this series is about merge-strategies. > * Inserted a new patch 8 that strikes some misleading or at least > no-longer-important text from git-rebase.txt (due changes back in late > 2006). Looking good. Will queue, together with the "switch" patches on top. Thanks.
On 8/4/2021 2:12 AM, Junio C Hamano wrote: > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes: > >> Changes since v1: >> >> * Multiple tweaks suggested by Eric, Dscho, and Junio >> * Removed patch 7 explaining no-renames since that probably belongs in git >> diff --no-renames instead, and this series is about merge-strategies. >> * Inserted a new patch 8 that strikes some misleading or at least >> no-longer-important text from git-rebase.txt (due changes back in late >> 2006). > > Looking good. > Will queue, together with the "switch" patches on top. Chiming in to say I like this version, too. Thanks! -Stolee
Hi Elijah, On Wed, 4 Aug 2021, Elijah Newren via GitGitGadget wrote: > Range-diff vs v1: > [...] This looks good to me. Thank you for making this a very pleasant review, Dscho
I noticed while updating my switch-default-merge-strategy-to-ort submission, that many of the changes were good documentation updates that we might want for Git v2.33.0. So I pulled those changes out and split them into lots of little commits so that if any parts need discussion or are objectionable, we can just drop those from this series and apply the rest for v2.33.0. The first 9 commits are just small documentation updates, but there is one commit at the end that updates an error message and a code comment. Changes since v1: * Multiple tweaks suggested by Eric, Dscho, and Junio * Removed patch 7 explaining no-renames since that probably belongs in git diff --no-renames instead, and this series is about merge-strategies. * Inserted a new patch 8 that strikes some misleading or at least no-longer-important text from git-rebase.txt (due changes back in late 2006). Elijah Newren (10): git-rebase.txt: correct antiquated claims about --rebase-merges directory-rename-detection.txt: small updates due to merge-ort optimizations Documentation: edit awkward references to `git merge-recursive` merge-strategies.txt: update wording for the resolve strategy merge-strategies.txt: do not imply using copy detection is desired merge-strategies.txt: avoid giving special preference to patience algorithm merge-strategies.txt: fix simple capitalization error git-rebase.txt: correct out-of-date and misleading text about renames merge-strategies.txt: add coverage of the `ort` merge strategy Update error message and code comment Documentation/git-rebase.txt | 27 ++++++----- Documentation/merge-options.txt | 4 +- Documentation/merge-strategies.txt | 48 +++++++++++-------- .../technical/directory-rename-detection.txt | 14 +++--- builtin/merge.c | 2 +- sequencer.c | 2 +- 6 files changed, 55 insertions(+), 42 deletions(-) base-commit: 66262451ec94d30ac4b80eb3123549cf7a788afd Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1059%2Fnewren%2Fort-doc-updates-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1059/newren/ort-doc-updates-v2 Pull-Request: https://github.com/git/git/pull/1059 Range-diff vs v1: 1: ab2367594a3 ! 1: 34352397168 git-rebase.txt: correct antiquated claims about --rebase-merges @@ Commit message 2019-07-31). However, when the limitation was lifted, the documentation was not updated. Update it now. + Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Elijah Newren <newren@gmail.com> ## Documentation/git-rebase.txt ## 2: 6b89ab8d9b1 = 2: 3fdd068231a directory-rename-detection.txt: small updates due to merge-ort optimizations 3: c1d056f0794 ! 3: 2a38320c2be Documentation: edit awkward references to `git merge-recursive` @@ Commit message ## Documentation/git-rebase.txt ## @@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below. + -s <strategy>:: --strategy=<strategy>:: - Use the given merge strategy. +- Use the given merge strategy. - If there is no `-s` option 'git merge-recursive' is used - instead. This implies --merge. -+ If there is no `-s` option the `recursive` strategy is the -+ default. This implies --merge. ++ Use the given merge strategy, instead of the default ++ `recursive`. This implies `--merge`. + Because 'git rebase' replays each commit from the working branch on top of the <upstream> branch using the given strategy, using 4: 3989f194ba9 ! 4: e422a1bc7d4 merge-strategies.txt: update wording for the resolve strategy @@ Metadata ## Commit message ## merge-strategies.txt: update wording for the resolve strategy - The resolve merge strategy was given prominent positioning in this - document, being listed first since it was the default at the time the - document was written. It hasn't been the default since before Git v1.0 - was released, though. Move it later in the document, near `octopus` and - `ours`. + It is probably helpful to cover the default merge strategy first, so + move the text for the resolve strategy to later in the document. Further, the wording for "resolve" claimed that it was "considered - generally safe and fast", which implies that the other strategies are - not. While such an implication may have been true in 2005 when written, - it may well be that `ort` is faster today (since it does not need to - recurse into all directories). Also, since `resolve` was the default - for less than a year while `recursive` has been the default for a decade - and a half, I think `recursive` is more battle-tested than `resolve` is. - Just strike this extraneous phrase. - - Also, provide some quick historical context that may help users - understand its purpose and place in the list of merge strategies. + generally safe and fast", which might imply in some readers minds that + the same is not true of other strategies. Rather than adding this text + to all the strategies, just remove it from this one. Signed-off-by: Elijah Newren <newren@gmail.com> @@ Documentation/merge-strategies.txt: subtree[=<path>];; + This can only resolve two heads (i.e. the current branch + and another branch you pulled from) using a 3-way merge + algorithm. It tries to carefully detect criss-cross -+ merge ambiguities. It cannot handle renames. This was -+ the default merge algorithm prior to November 2005. ++ merge ambiguities. It does not handle renames. + octopus:: This resolves cases with more than two heads, but refuses to do 5: 5f974afe47c = 5: b1db5fdebe5 merge-strategies.txt: do not imply using copy detection is desired 6: 6116f4750fd ! 6: 44101062e0e merge-strategies.txt: avoid giving special preference to patience algorithm @@ Documentation/merge-strategies.txt: theirs;; - matching lines (e.g., braces from distinct functions). Use - this when the branches to be merged have diverged wildly. - See also linkgit:git-diff[1] `--patience`. -+ Deprecated shorthand for diff-algorithm=patience. ++ Deprecated synonym for `diff-algorithm=patience`. diff-algorithm=[patience|minimal|histogram|myers];; Use a different diff algorithm while merging, which can help 7: 7eecf879d60 < -: ----------- merge-strategies.txt: explain why no-renames might be useful 8: 010702d0841 = 7: d1521f98dee merge-strategies.txt: fix simple capitalization error -: ----------- > 8: 8978132397e git-rebase.txt: correct out-of-date and misleading text about renames 9: 37a69fd2e0b ! 9: bc92826f7e5 Documentation: add coverage of the `ort` merge strategy @@ Metadata Author: Elijah Newren <newren@gmail.com> ## Commit message ## - Documentation: add coverage of the `ort` merge strategy + merge-strategies.txt: add coverage of the `ort` merge strategy Signed-off-by: Elijah Newren <newren@gmail.com> - ## Documentation/git-rebase.txt ## -@@ Documentation/git-rebase.txt: See also INCOMPATIBLE OPTIONS below. - - -m:: - --merge:: -- Use merging strategies to rebase. When the recursive (default) merge -- strategy is used, this allows rebase to be aware of renames on the -- upstream side. This is the default. -+ Use merging strategies to rebase. When either the `recursive` -+ (default) or `ort` merge strategy is used, this allows rebase -+ to be aware of renames on the upstream side. This is the -+ default. - + - Note that a rebase merge works by replaying each commit from the working - branch on top of the <upstream> branch. Because of this, when a merge - ## Documentation/merge-strategies.txt ## @@ Documentation/merge-strategies.txt: subtree[=<path>];; is prefixed (or stripped from the beginning) to make the shape of @@ Documentation/merge-strategies.txt: subtree[=<path>];; +However, it ignores three of those options: `no-renames`, +`patience` and `diff-algorithm`. It always runs with rename +detection (it handles it much faster than `recursive` does), and -+it specifically uses diff-algorithm=histogram. ++it specifically uses `diff-algorithm=histogram`. + resolve:: This can only resolve two heads (i.e. the current branch 10: 2a7169c8c1b ! 10: 4a78ac53424 Update error message and code comment @@ builtin/merge.c: static int try_merge_strategy(const char *strategy, struct comm for (x = 0; x < xopts_nr; x++) if (parse_merge_opt(&o, xopts[x])) - die(_("Unknown option for merge-recursive: -X%s"), xopts[x]); -+ die(_("Unknown strategy option: -X%s"), xopts[x]); ++ die(_("unknown strategy option: -X%s"), xopts[x]); o.branch1 = head_arg; o.branch2 = merge_remote_util(remoteheads->item)->name;