* [PATCH] merge-options.txt: clarify meaning of various ff-related options @ 2019-08-28 0:13 Elijah Newren 2019-08-28 9:05 ` Sergey Organov 0 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2019-08-28 0:13 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Elijah Newren As discovered on the mailing list, some of the descriptions of the ff-related options were unclear. Try to be more precise with what these options do. Signed-off-by: Elijah Newren <newren@gmail.com> --- I noticed this patch sitting around in one of my branches, and noticed it wasn't upstream. I'm pretty sure I submitted it a few months back, but I think it got lost in the cracks. Resubmitting and I'll see if I can do a better job following up on it. Documentation/merge-options.txt | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 79a00d2a4a..b39df5f126 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -40,20 +40,22 @@ set to `no` at the beginning of them. case of a merge conflict. --ff:: - When the merge resolves as a fast-forward, only update the branch - pointer, without creating a merge commit. This is the default + When the merge can resolve as a fast-forward, do so (only + update the branch pointer to match the merged branch; do not + create a merge commit). When a fast forward update is not + possible, create a merge commit. This is the default behavior. --no-ff:: - Create a merge commit even when the merge resolves as a - fast-forward. This is the default behaviour when merging an - annotated (and possibly signed) tag that is not stored in - its natural place in 'refs/tags/' hierarchy. + Create a merge commit even when the merge could instead resolve + as a fast-forward. This is the default behaviour when merging + an annotated (and possibly signed) tag that is not stored in its + natural place in 'refs/tags/' hierarchy. --ff-only:: - Refuse to merge and exit with a non-zero status unless the - current `HEAD` is already up to date or the merge can be - resolved as a fast-forward. + When possible, resolve the merge as a fast-forward (do not + create a merge commit). When not possible, refuse to merge and + exit with a non-zero status. -S[<keyid>]:: --gpg-sign[=<keyid>]:: -- 2.23.0.38.gfc6987be7e ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 0:13 [PATCH] merge-options.txt: clarify meaning of various ff-related options Elijah Newren @ 2019-08-28 9:05 ` Sergey Organov 2019-08-28 15:51 ` [PATCH v2] " Elijah Newren 0 siblings, 1 reply; 16+ messages in thread From: Sergey Organov @ 2019-08-28 9:05 UTC (permalink / raw) To: Elijah Newren; +Cc: git, Junio C Hamano Elijah Newren <newren@gmail.com> writes: > As discovered on the mailing list, some of the descriptions of the > ff-related options were unclear. Try to be more precise with what these > options do. > > Signed-off-by: Elijah Newren <newren@gmail.com> > --- > I noticed this patch sitting around in one of my branches, and noticed it > wasn't upstream. I'm pretty sure I submitted it a few months back, but I > think it got lost in the cracks. Resubmitting and I'll see if I can do a > better job following up on it. > > Documentation/merge-options.txt | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt > index 79a00d2a4a..b39df5f126 100644 > --- a/Documentation/merge-options.txt > +++ b/Documentation/merge-options.txt > @@ -40,20 +40,22 @@ set to `no` at the beginning of them. > case of a merge conflict. > > --ff:: > - When the merge resolves as a fast-forward, only update the branch > - pointer, without creating a merge commit. This is the default > + When the merge can resolve as a fast-forward, do so (only > + update the branch pointer to match the merged branch; do not > + create a merge commit). When a fast forward update is not > + possible, create a merge commit. This is the default > behavior. > > --no-ff:: > - Create a merge commit even when the merge resolves as a > - fast-forward. This is the default behaviour when merging an > - annotated (and possibly signed) tag that is not stored in > - its natural place in 'refs/tags/' hierarchy. > + Create a merge commit even when the merge could instead resolve > + as a fast-forward. This is the default behaviour when merging > + an annotated (and possibly signed) tag that is not stored in its > + natural place in 'refs/tags/' hierarchy. Please notice that virtually all the other cases of --something/--no-something are formatted like this: --something:: --no-something:: [descriptions] So, even only for consistency, it seems to be better to have this the same way: --ff:: --no-ff:: --ff-only:: [descriptions] that, as a bonus, will make it explicit and crystal clear that these 3 things are alternatives, and thus the last one on the command line takes precedence. -- Sergey ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 9:05 ` Sergey Organov @ 2019-08-28 15:51 ` Elijah Newren 2019-08-28 18:45 ` Martin Ågren 0 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2019-08-28 15:51 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Sergey Organov, Elijah Newren As discovered on the mailing list, some of the descriptions of the ff-related options were unclear. Try to be more precise with what these options do. Signed-off-by: Elijah Newren <newren@gmail.com> --- Changes since v1: * Grouped much like --option/--no-option items are to make it clearer that these are related options, as suggested by Sergey. Documentation/merge-options.txt | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 79a00d2a4a..348d66f54b 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -39,21 +39,28 @@ set to `no` at the beginning of them. to `MERGE_MSG` before being passed on to the commit machinery in the case of a merge conflict. ---ff:: - When the merge resolves as a fast-forward, only update the branch - pointer, without creating a merge commit. This is the default - behavior. - +--ff-only:: --no-ff:: - Create a merge commit even when the merge resolves as a - fast-forward. This is the default behaviour when merging an - annotated (and possibly signed) tag that is not stored in - its natural place in 'refs/tags/' hierarchy. +--ff:: + Whether to only allow resolving the merge as a fast forward + (only updating the branch pointer to match the merged branch + and not creating a merge commit), to never allow it (always + creating a merge commit), or to prefer it when possible. The + default is --ff, except when merging an annotated (and + possibly signed) tag that is not stored in its natural place + in 'refs/tags/' hierarchy (in which case --no-ff is the + default). ++ +With --ff-only, resolve the merge as a fast-forward when possible +(when the merged branch contains the current branch in its history). +When not possible, refuse to merge and exit with a non-zero status. ++ +With --no-ff, create a merge commit in all cases, even when the merge +could instead resolve as a fast-forward. ++ +With --ff, resolve the merge as a fast-forward when possible. When not +possible, create a merge commit. ---ff-only:: - Refuse to merge and exit with a non-zero status unless the - current `HEAD` is already up to date or the merge can be - resolved as a fast-forward. -S[<keyid>]:: --gpg-sign[=<keyid>]:: -- 2.23.0.9.g568fda2d03 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 15:51 ` [PATCH v2] " Elijah Newren @ 2019-08-28 18:45 ` Martin Ågren 2019-08-28 19:15 ` Sergey Organov ` (2 more replies) 0 siblings, 3 replies; 16+ messages in thread From: Martin Ågren @ 2019-08-28 18:45 UTC (permalink / raw) To: Elijah Newren; +Cc: Junio C Hamano, Git Mailing List, Sergey Organov Hi Elijah, On Wed, 28 Aug 2019 at 17:52, Elijah Newren <newren@gmail.com> wrote: > ---ff:: > - When the merge resolves as a fast-forward, only update the branch > - pointer, without creating a merge commit. This is the default > - behavior. > - > +--ff-only:: > --no-ff:: > - Create a merge commit even when the merge resolves as a > - fast-forward. This is the default behaviour when merging an > - annotated (and possibly signed) tag that is not stored in > - its natural place in 'refs/tags/' hierarchy. > +--ff:: > + Whether to only allow resolving the merge as a fast forward > + (only updating the branch pointer to match the merged branch > + and not creating a merge commit), to never allow it (always > + creating a merge commit), or to prefer it when possible. The > + default is --ff, except when merging an annotated (and It would be great if you'd write this as `--ff` so that it got monospaced in the rendered documentation. Same thing with `no-ff` later in this paragraph and a few more times in the next three paragraphs that you're adding. > + possibly signed) tag that is not stored in its natural place > + in 'refs/tags/' hierarchy (in which case --no-ff is the > + default). I'd also write `refs/tags/`, but I realize that you're just inheriting what is already here. If you'd rather punt on that, that's understood. This whole document could need a look-over with respect to monospacing anyway, but it seems unfortunate to introduce *new* non-monospaced instances, especially for command-line options where it's pretty clear how they should be handled (Documentation/CodingGuidelines, line ~610). > ++ > +With --ff-only, resolve the merge as a fast-forward when possible > +(when the merged branch contains the current branch in its history). > +When not possible, refuse to merge and exit with a non-zero status. > ++ > +With --no-ff, create a merge commit in all cases, even when the merge > +could instead resolve as a fast-forward. > ++ > +With --ff, resolve the merge as a fast-forward when possible. When not > +possible, create a merge commit. > > ---ff-only:: > - Refuse to merge and exit with a non-zero status unless the > - current `HEAD` is already up to date or the merge can be > - resolved as a fast-forward. I was sort of expecting these to be listed in the order "--ff, --no-ff, --ff-only", and I see Sergey suggested the same ordering. The way your proposed text reads does make sense though... Would it read as well turning it over and going through the options in the other order? That's the way it is before your patch, so you could argue "but people don't grok that!". What your patch does well is to offer an overview before describing each option in a bit more detail. Would that work with the reversed direction as well (compared to this patch)? Dunno. I wondered briefly whether it might make sense to float "The default is `--no-ff`." to the top, but since it's really "The default ... unless so-and-so", it would probably complicate things more than it'd help. Martin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 18:45 ` Martin Ågren @ 2019-08-28 19:15 ` Sergey Organov 2019-08-28 19:53 ` Martin Ågren 2019-08-28 22:51 ` Elijah Newren 2019-08-28 22:57 ` [PATCH v3] " Elijah Newren 2019-08-30 19:45 ` [PATCH v2] " Junio C Hamano 2 siblings, 2 replies; 16+ messages in thread From: Sergey Organov @ 2019-08-28 19:15 UTC (permalink / raw) To: Martin Ågren; +Cc: Elijah Newren, Junio C Hamano, Git Mailing List Hi, Martin Ågren <martin.agren@gmail.com> writes: [...] >> ++ >> +With --ff-only, resolve the merge as a fast-forward when possible >> +(when the merged branch contains the current branch in its history). >> +When not possible, refuse to merge and exit with a non-zero status. >> ++ >> +With --no-ff, create a merge commit in all cases, even when the merge >> +could instead resolve as a fast-forward. >> ++ >> +With --ff, resolve the merge as a fast-forward when possible. When not >> +possible, create a merge commit. >> >> ---ff-only:: >> - Refuse to merge and exit with a non-zero status unless the >> - current `HEAD` is already up to date or the merge can be >> - resolved as a fast-forward. > > I was sort of expecting these to be listed in the order "--ff, --no-ff, > --ff-only", and I see Sergey suggested the same ordering. The way your > proposed text reads does make sense though... Would it read as well > turning it over and going through the options in the other order? That's > the way it is before your patch, so you could argue "but people don't > grok that!". What your patch does well is to offer an overview before > describing each option in a bit more detail. Would that work with the > reversed direction as well (compared to this patch)? Dunno. > > I wondered briefly whether it might make sense to float "The default is > `--no-ff`." to the top, but since it's really "The default ... unless > so-and-so", it would probably complicate things more than it'd help. Dunno if it helps, but here is what I came up with somewhere in previous discussions: --ff:: --no-ff:: --ff-only:: When the merge resolves as a fast-forward, only update the branch pointer (without creating a merge commit). When a fast forward update is not possible, create a merge commit. This is the default behavior, unless merging an annotated (and possibly signed) tag that is not stored in its natural place in 'refs/tags/' hierarchy, in which case --no-ff is assumed. + With --no-ff create a merge commit even when the merge could instead resolve as a fast-forward. + With --ff-only resolve the merge as a fast-forward (never create a merge commit). When fast-forward is not possible, refuse to merge and exit with non-zero status. -- Sergey ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 19:15 ` Sergey Organov @ 2019-08-28 19:53 ` Martin Ågren 2019-08-29 9:35 ` Sergey Organov 2019-08-28 22:51 ` Elijah Newren 1 sibling, 1 reply; 16+ messages in thread From: Martin Ågren @ 2019-08-28 19:53 UTC (permalink / raw) To: Sergey Organov; +Cc: Elijah Newren, Junio C Hamano, Git Mailing List Hiya, On Wed, 28 Aug 2019 at 21:15, Sergey Organov <sorganov@gmail.com> wrote: > > I was sort of expecting these to be listed in the order "--ff, --no-ff, > > --ff-only", and I see Sergey suggested the same ordering. The way your > > proposed text reads does make sense though... Would it read as well > > turning it over and going through the options in the other order? That's > > the way it is before your patch, so you could argue "but people don't > > grok that!". What your patch does well is to offer an overview before > > describing each option in a bit more detail. Would that work with the > > reversed direction as well (compared to this patch)? Dunno. > Dunno if it helps, but here is what I came up with somewhere in previous > discussions: > > --ff:: > --no-ff:: > --ff-only:: > When the merge resolves as a fast-forward, only update the > branch pointer (without creating a merge commit). When a fast > forward update is not possible, create a merge commit. This is > the default behavior, unless merging an annotated (and possibly > signed) tag that is not stored in its natural place in > 'refs/tags/' hierarchy, in which case --no-ff is assumed. > + > With --no-ff create a merge commit even when the merge could instead > resolve as a fast-forward. > + > With --ff-only resolve the merge as a fast-forward (never create a merge > commit). When fast-forward is not possible, refuse to merge and exit > with non-zero status. I think this could make clear that the first paragraph talks about `--ff`. I know that we often list `--foo` and `--no-foo`, say "This option does bar" and leave it to the reader to figure out which is which. Most of the time, that's fairly obvious though (foo=bar). With this tri-state option, we might be past the point of fair obviousness. Then again, seeing "ff" and "fast forward", it's not /that hard/ to figure out. Apart from that, this reads well. Of course with the usual caveat of the topic being fresh on my mind, so in general, I'd be more likely to understand what it is I'm reading. ;-) Martin ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 19:53 ` Martin Ågren @ 2019-08-29 9:35 ` Sergey Organov 0 siblings, 0 replies; 16+ messages in thread From: Sergey Organov @ 2019-08-29 9:35 UTC (permalink / raw) To: Martin Ågren; +Cc: Elijah Newren, Junio C Hamano, Git Mailing List Martin Ågren <martin.agren@gmail.com> writes: > Hiya, This one was new to me :-) > > On Wed, 28 Aug 2019 at 21:15, Sergey Organov <sorganov@gmail.com> wrote: >> > I was sort of expecting these to be listed in the order "--ff, --no-ff, >> > --ff-only", and I see Sergey suggested the same ordering. The way your >> > proposed text reads does make sense though... Would it read as well >> > turning it over and going through the options in the other order? That's >> > the way it is before your patch, so you could argue "but people don't >> > grok that!". What your patch does well is to offer an overview before >> > describing each option in a bit more detail. Would that work with the >> > reversed direction as well (compared to this patch)? Dunno. > >> Dunno if it helps, but here is what I came up with somewhere in previous >> discussions: >> >> --ff:: >> --no-ff:: >> --ff-only:: >> When the merge resolves as a fast-forward, only update the >> branch pointer (without creating a merge commit). When a fast >> forward update is not possible, create a merge commit. This is >> the default behavior, unless merging an annotated (and possibly >> signed) tag that is not stored in its natural place in >> 'refs/tags/' hierarchy, in which case --no-ff is assumed. >> + >> With --no-ff create a merge commit even when the merge could instead >> resolve as a fast-forward. >> + >> With --ff-only resolve the merge as a fast-forward (never create a merge >> commit). When fast-forward is not possible, refuse to merge and exit >> with non-zero status. > > I think this could make clear that the first paragraph talks about > `--ff`. I know that we often list `--foo` and `--no-foo`, say "This > option does bar" and leave it to the reader to figure out which is which. > Most of the time, that's fairly obvious though (foo=bar). With this > tri-state option, we might be past the point of fair obviousness. > Then again, seeing "ff" and "fast forward", it's not /that hard/ to > figure out. Yeah, I've noticed that myself, but decided to keep it, as this was meant to be a quick re-arrangement of already existing description, and be as short as at all possible, as lengthy descriptions tend to obscure problems. Being this way it shows how difficult it is to document flawed design. I guess the author of --ff-only would think twice should --ff/--no-ff be originally documented similarly to the rest of 2-way options. At least he'd likely call it --only-ff, as: --ff:: --no-ff:: --only-ff:: looks much better. Yeah, I do realize all this is historical and such... > > Apart from that, this reads well. Of course with the usual caveat of > the topic being fresh on my mind, so in general, I'd be more likely to > understand what it is I'm reading. ;-) ... and I'm in even worse position here ;-) -- Sergey ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 19:15 ` Sergey Organov 2019-08-28 19:53 ` Martin Ågren @ 2019-08-28 22:51 ` Elijah Newren 2019-08-29 9:15 ` Sergey Organov 1 sibling, 1 reply; 16+ messages in thread From: Elijah Newren @ 2019-08-28 22:51 UTC (permalink / raw) To: Sergey Organov; +Cc: Martin Ågren, Junio C Hamano, Git Mailing List On Wed, Aug 28, 2019 at 12:15 PM Sergey Organov <sorganov@gmail.com> wrote: > > Hi, > [...] > Dunno if it helps, but here is what I came up with somewhere in previous > discussions: > > --ff:: > --no-ff:: > --ff-only:: > When the merge resolves as a fast-forward, only update the I think this loose wording (that you just took from the original) is problematic. Saying that a "merge resolves as a fast-forward" seems to imply that there are circumstances when a fast-forward is the only option. An _individual_ can decide to resolve a merge as a fast-forward in some circumstances, but it's certainly not the only choice in any circumstance. If you want to keep this wording short, you could replace "resolves" with "can be resolved". > branch pointer (without creating a merge commit). When a fast Only update the branch pointer to what? (Yes, I know the original text we were improving left this unclear, but it's worth noting.) > forward update is not possible, create a merge commit. This is > the default behavior, unless merging an annotated (and possibly > signed) tag that is not stored in its natural place in > 'refs/tags/' hierarchy, in which case --no-ff is assumed. Maybe it's just me, but I think it takes extra human cycles to figure out that this paragraph is referring just to the --ff case, and that users might not be able to do so until after reading the next 2-3 sentences. While more brief, I think it will cause people to need to read the description for these three options twice, removing most the savings from being shorter. It'd be better if it could be re-worded to not need re-reads. > + > With --no-ff create a merge commit even when the merge could instead > resolve as a fast-forward. > + > With --ff-only resolve the merge as a fast-forward (never create a merge > commit). When fast-forward is not possible, refuse to merge and exit > with non-zero status. Something else I was trying to address with my patch that perhaps you can see a different way to tackle: Using the wording "when possible" is probably going to make users wonder when a fast forward is possible; the "can be resolved" wording tweak also makes it more likely they will wonder about this. Another question they will be wondering about is what a fast forward is (which you partially explain). Some basic knowledge of both are probably very useful in helping them decide which option to actually pick. As such, I think trying to explain the answers to these sub-questions will assist them in knowing which option to use. Simply inserting a couple phrases (e.g. "when the merged branch contains the current branch in its history", and "only update the branch pointer *to match the merged branch* and do not create a merge commit") may help a lot. Anyway, I'll send a v3 addressing Martin's comments; if you've got further suggestions for streamlining or rearranging, though, please do send them along. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 22:51 ` Elijah Newren @ 2019-08-29 9:15 ` Sergey Organov 0 siblings, 0 replies; 16+ messages in thread From: Sergey Organov @ 2019-08-29 9:15 UTC (permalink / raw) To: Elijah Newren; +Cc: Martin Ågren, Junio C Hamano, Git Mailing List Elijah Newren <newren@gmail.com> writes: > On Wed, Aug 28, 2019 at 12:15 PM Sergey Organov <sorganov@gmail.com> wrote: >> >> Hi, >> > [...] >> Dunno if it helps, but here is what I came up with somewhere in previous >> discussions: >> >> --ff:: >> --no-ff:: >> --ff-only:: >> When the merge resolves as a fast-forward, only update the > > I think this loose wording (that you just took from the original) is > problematic. Saying that a "merge resolves as a fast-forward" seems > to imply that there are circumstances when a fast-forward is the only > option. An _individual_ can decide to resolve a merge as a > fast-forward in some circumstances, but it's certainly not the only > choice in any circumstance. If you want to keep this wording short, > you could replace "resolves" with "can be resolved". > >> branch pointer (without creating a merge commit). When a fast > > Only update the branch pointer to what? (Yes, I know the original > text we were improving left this unclear, but it's worth noting.) > >> forward update is not possible, create a merge commit. This is >> the default behavior, unless merging an annotated (and possibly >> signed) tag that is not stored in its natural place in >> 'refs/tags/' hierarchy, in which case --no-ff is assumed. > > Maybe it's just me, but I think it takes extra human cycles to figure > out that this paragraph is referring just to the --ff case, and that > users might not be able to do so until after reading the next 2-3 > sentences. While more brief, I think it will cause people to need to > read the description for these three options twice, removing most the > savings from being shorter. It'd be better if it could be re-worded > to not need re-reads. > >> + >> With --no-ff create a merge commit even when the merge could instead >> resolve as a fast-forward. >> + >> With --ff-only resolve the merge as a fast-forward (never create a merge >> commit). When fast-forward is not possible, refuse to merge and exit >> with non-zero status. > > Something else I was trying to address with my patch that perhaps you > can see a different way to tackle: Using the wording "when possible" > is probably going to make users wonder when a fast forward is > possible; the "can be resolved" wording tweak also makes it more > likely they will wonder about this. Another question they will be > wondering about is what a fast forward is (which you partially > explain). Some basic knowledge of both are probably very useful in > helping them decide which option to actually pick. As such, I think > trying to explain the answers to these sub-questions will assist them > in knowing which option to use. Simply inserting a couple phrases > (e.g. "when the merged branch contains the current branch in its > history", and "only update the branch pointer *to match the merged > branch* and do not create a merge commit") may help a lot. > > Anyway, I'll send a v3 addressing Martin's comments; if you've got > further suggestions for streamlining or rearranging, though, please do > send them along. Thanks for thorough reply! My version was meant to show how to re-arrange the description preserving original wording as much as possible, so your version should be better, as it addresses other problems as well. -- Sergey ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 18:45 ` Martin Ågren 2019-08-28 19:15 ` Sergey Organov @ 2019-08-28 22:57 ` Elijah Newren 2019-08-30 19:57 ` Junio C Hamano 2019-08-30 19:45 ` [PATCH v2] " Junio C Hamano 2 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2019-08-28 22:57 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Sergey Organov, Martin Ågren, Elijah Newren As discovered on the mailing list, some of the descriptions of the ff-related options were unclear. Try to be more precise with what these options do. Signed-off-by: Elijah Newren <newren@gmail.com> --- Changes since v2: * reordered the options * typeset the option flags differently to ensure they are monospace in the rendered documentation Documentation/merge-options.txt | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 79a00d2a4a..ed3804650b 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -40,20 +40,26 @@ set to `no` at the beginning of them. case of a merge conflict. --ff:: - When the merge resolves as a fast-forward, only update the branch - pointer, without creating a merge commit. This is the default - behavior. - --no-ff:: - Create a merge commit even when the merge resolves as a - fast-forward. This is the default behaviour when merging an - annotated (and possibly signed) tag that is not stored in - its natural place in 'refs/tags/' hierarchy. - --ff-only:: - Refuse to merge and exit with a non-zero status unless the - current `HEAD` is already up to date or the merge can be - resolved as a fast-forward. + Whether to prefer resolving the merge as a fast forward (only + updating the branch pointer to match the merged branch and not + creating a merge commit), to never allow it (always creating a + merge commit), or to only allow fast forward updates. The + default is `--ff`, except when merging an annotated (and + possibly signed) tag that is not stored in its natural place + in the `refs/tags/` hierarchy (in which case `--no-ff` is + assumed). ++ +With `--ff`, resolve the merge as a fast-forward when possible (when the +merged branch contains the current branch in its history). When not +possible, create a merge commit. ++ +With `--no-ff`, create a merge commit in all cases, even when the merge +could instead be resolved as a fast-forward. ++ +With `--ff-only`, resolve the merge as a fast-forward when possible. +When not possible, refuse to merge and exit with a non-zero status. -S[<keyid>]:: --gpg-sign[=<keyid>]:: -- 2.23.0.3.g93492fb5e6.dirty ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 22:57 ` [PATCH v3] " Elijah Newren @ 2019-08-30 19:57 ` Junio C Hamano 2019-08-30 20:16 ` Eric Sunshine 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2019-08-30 19:57 UTC (permalink / raw) To: Elijah Newren; +Cc: git, Sergey Organov, Martin Ågren Elijah Newren <newren@gmail.com> writes: > diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt > index 79a00d2a4a..ed3804650b 100644 > --- a/Documentation/merge-options.txt > +++ b/Documentation/merge-options.txt > @@ -40,20 +40,26 @@ set to `no` at the beginning of them. > case of a merge conflict. > > --ff:: > --no-ff:: > --ff-only:: > + Whether to prefer resolving the merge as a fast forward (only > + updating the branch pointer to match the merged branch and not > + creating a merge commit), to never allow it (always creating a > + merge commit), or to only allow fast forward updates. The > + default is `--ff`, except when merging an annotated (and > + possibly signed) tag that is not stored in its natural place > + in the `refs/tags/` hierarchy (in which case `--no-ff` is > + assumed). > ++ > +With `--ff`, resolve the merge as a fast-forward when possible (when the > +merged branch contains the current branch in its history). When not > +possible, create a merge commit. > ++ > +With `--no-ff`, create a merge commit in all cases, even when the merge > +could instead be resolved as a fast-forward. > ++ > +With `--ff-only`, resolve the merge as a fast-forward when possible. > +When not possible, refuse to merge and exit with a non-zero status. I cannot shake the feeling that the above redundantly repeats the same thing twice. If we want to dedicate one paragraph for each of these options, we can and should make the introductory paragraph lighter by saying something like Specifies how a merge is handled when the merged-in history is already a descendant of the current history. `--ff` is the default unless merging an annotated or signed tag that is not stored in the `refs/tags/` hierarchy, in which case `--no-ff` is the default. Alternatively, we could sprinkle the actual option name in the first paragraph and drop the last three paragraphs, while fattening the description as necessary, e.g. Whether to prefer resolving the merge as a fast-forward and update the branch pointer to match the merged branch without creating an extra merge commit (`--ff`), never allow fast-forward and always creating an extra merge commit (`--no-ff`), or to only allow fast forward updates and reject when a merge commit needs to be created (`--ff-only`). The default is ... I think either approach shown above would reduce the redundancy. I do not care too deeply which one of these approaches is used myself, but the redundancy feels a bit disturbing. Thanks. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] merge-options.txt: clarify meaning of various ff-related options 2019-08-30 19:57 ` Junio C Hamano @ 2019-08-30 20:16 ` Eric Sunshine 2019-08-31 0:23 ` [PATCH v4] " Elijah Newren 0 siblings, 1 reply; 16+ messages in thread From: Eric Sunshine @ 2019-08-30 20:16 UTC (permalink / raw) To: Junio C Hamano; +Cc: Elijah Newren, Git List, Sergey Organov, Martin Ågren On Fri, Aug 30, 2019 at 3:57 PM Junio C Hamano <gitster@pobox.com> wrote: > If we want to dedicate one paragraph for each of these options, we > can and should make the introductory paragraph lighter by saying > something like > > Specifies how a merge is handled when the merged-in history > is already a descendant of the current history. `--ff` is > the default unless merging an annotated or signed tag that > is not stored in the `refs/tags/` hierarchy, in which case > `--no-ff` is the default. > > Alternatively, we could sprinkle the actual option name in the first > paragraph and drop the last three paragraphs, while fattening the > description as necessary, e.g. > > Whether to prefer resolving the merge as a fast-forward and > update the branch pointer to match the merged branch without > creating an extra merge commit (`--ff`), never allow fast-forward > and always creating an extra merge commit (`--no-ff`), or to > only allow fast forward updates and reject when a merge > commit needs to be created (`--ff-only`). The default is ... > > I think either approach shown above would reduce the redundancy. I > do not care too deeply which one of these approaches is used myself, > but the redundancy feels a bit disturbing. I have not been paying close attention to this thread, but upon reading your suggested rewrites, I find the lighter paragraph (the first of your options), followed by the three short paragraphs -- each dedicated to a distinct option -- easier to follow and grok. I think that's because the lighter/shorter arrangement keeps the three cases reasonably separate -- thus the reader is able to absorb and understand each distinct option in isolation -- rather than having to manually pluck out the meaning of each option from one long, run-on sentence. ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] merge-options.txt: clarify meaning of various ff-related options 2019-08-30 20:16 ` Eric Sunshine @ 2019-08-31 0:23 ` Elijah Newren 0 siblings, 0 replies; 16+ messages in thread From: Elijah Newren @ 2019-08-31 0:23 UTC (permalink / raw) To: git Cc: Junio C Hamano, Sergey Organov, Martin Ågren, Eric Sunshine, Elijah Newren As discovered on the mailing list, some of the descriptions of the ff-related options were unclear. Try to be more precise with what these options do. Signed-off-by: Elijah Newren <newren@gmail.com> --- Changes since v3: * Shorten the leading paragraph, as suggested by Junio (and Eric). Documentation/merge-options.txt | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 79a00d2a4a..94be34f941 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -40,20 +40,24 @@ set to `no` at the beginning of them. case of a merge conflict. --ff:: - When the merge resolves as a fast-forward, only update the branch - pointer, without creating a merge commit. This is the default - behavior. - --no-ff:: - Create a merge commit even when the merge resolves as a - fast-forward. This is the default behaviour when merging an - annotated (and possibly signed) tag that is not stored in - its natural place in 'refs/tags/' hierarchy. - --ff-only:: - Refuse to merge and exit with a non-zero status unless the - current `HEAD` is already up to date or the merge can be - resolved as a fast-forward. + Specifies how a merge is handled when the merged-in history is + already a descendant of the current history. `--ff` is the + default unless merging an annotated (and possibly signed) tag + that is not stored in its natural place in the `refs/tags/` + hierarchy, in which case `--no-ff` is assumed. ++ +With `--ff`, when possible resolve the merge as a fast-forward (only +update the branch pointer to match the merged branch; do not create a +merge commit). When not possible (when the merged-in history is not a +descendant of the current history), create a merge commit. ++ +With `--no-ff`, create a merge commit in all cases, even when the merge +could instead be resolved as a fast-forward. ++ +With `--ff-only`, resolve the merge as a fast-forward when possible. +When not possible, refuse to merge and exit with a non-zero status. -S[<keyid>]:: --gpg-sign[=<keyid>]:: -- 2.23.0.39.gf92d9de5c3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-28 18:45 ` Martin Ågren 2019-08-28 19:15 ` Sergey Organov 2019-08-28 22:57 ` [PATCH v3] " Elijah Newren @ 2019-08-30 19:45 ` Junio C Hamano 2019-08-30 19:48 ` Elijah Newren 2 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2019-08-30 19:45 UTC (permalink / raw) To: Martin Ågren; +Cc: Elijah Newren, Git Mailing List, Sergey Organov Martin Ågren <martin.agren@gmail.com> writes: >> +--ff:: >> + Whether to only allow resolving the merge as a fast forward >> + (only updating the branch pointer to match the merged branch >> + and not creating a merge commit), to never allow it (always >> + creating a merge commit), or to prefer it when possible. The >> + default is --ff, except when merging an annotated (and > > It would be great if you'd write this as `--ff` so that it got > monospaced in the rendered documentation. ... > I'd also write `refs/tags/`, but I realize that you're just inheriting > what is already here. ... These comments may sound nitpicky at times, and some parts may be beyond the scope of the discussion on the patch and are better left for a later time, but the consistency is valuable. Thanks for mentioning them; and it would be appreciated if these suggesions are followed through after the dust settles. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-30 19:45 ` [PATCH v2] " Junio C Hamano @ 2019-08-30 19:48 ` Elijah Newren 2019-08-30 20:27 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Elijah Newren @ 2019-08-30 19:48 UTC (permalink / raw) To: Junio C Hamano; +Cc: Martin Ågren, Git Mailing List, Sergey Organov On Fri, Aug 30, 2019 at 12:45 PM Junio C Hamano <gitster@pobox.com> wrote: > > Martin Ågren <martin.agren@gmail.com> writes: > > >> +--ff:: > >> + Whether to only allow resolving the merge as a fast forward > >> + (only updating the branch pointer to match the merged branch > >> + and not creating a merge commit), to never allow it (always > >> + creating a merge commit), or to prefer it when possible. The > >> + default is --ff, except when merging an annotated (and > > > > It would be great if you'd write this as `--ff` so that it got > > monospaced in the rendered documentation. ... > > I'd also write `refs/tags/`, but I realize that you're just inheriting > > what is already here. ... > > These comments may sound nitpicky at times, and some parts may be > beyond the scope of the discussion on the patch and are better left > for a later time, but the consistency is valuable. > > Thanks for mentioning them; and it would be appreciated if these > suggesions are followed through after the dust settles. I'm confused; these suggestions were incorporated into V3 already. Am I misunderstanding something here? (But yes, I agree that Martin's reviews are valuable.) ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] merge-options.txt: clarify meaning of various ff-related options 2019-08-30 19:48 ` Elijah Newren @ 2019-08-30 20:27 ` Junio C Hamano 0 siblings, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2019-08-30 20:27 UTC (permalink / raw) To: Elijah Newren; +Cc: Martin Ågren, Git Mailing List, Sergey Organov Elijah Newren <newren@gmail.com> writes: > On Fri, Aug 30, 2019 at 12:45 PM Junio C Hamano <gitster@pobox.com> wrote: >> >> Martin Ågren <martin.agren@gmail.com> writes: >> >> >> +--ff:: >> >> + Whether to only allow resolving the merge as a fast forward >> >> + (only updating the branch pointer to match the merged branch >> >> + and not creating a merge commit), to never allow it (always >> >> + creating a merge commit), or to prefer it when possible. The >> >> + default is --ff, except when merging an annotated (and >> > >> > It would be great if you'd write this as `--ff` so that it got >> > monospaced in the rendered documentation. ... >> > I'd also write `refs/tags/`, but I realize that you're just inheriting >> > what is already here. ... >> >> These comments may sound nitpicky at times, and some parts may be >> beyond the scope of the discussion on the patch and are better left >> for a later time, but the consistency is valuable. >> >> Thanks for mentioning them; and it would be appreciated if these >> suggesions are followed through after the dust settles. > > I'm confused; these suggestions were incorporated into V3 already. Am > I misunderstanding something here? I was assuming that outside the context of the patch there also are the same malformatted reference to refs/blah and --option "you're just inheriting", which would be cleaned up outside of this topic after the dust settles, if the suggestions are followed through. ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-08-31 0:23 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-28 0:13 [PATCH] merge-options.txt: clarify meaning of various ff-related options Elijah Newren 2019-08-28 9:05 ` Sergey Organov 2019-08-28 15:51 ` [PATCH v2] " Elijah Newren 2019-08-28 18:45 ` Martin Ågren 2019-08-28 19:15 ` Sergey Organov 2019-08-28 19:53 ` Martin Ågren 2019-08-29 9:35 ` Sergey Organov 2019-08-28 22:51 ` Elijah Newren 2019-08-29 9:15 ` Sergey Organov 2019-08-28 22:57 ` [PATCH v3] " Elijah Newren 2019-08-30 19:57 ` Junio C Hamano 2019-08-30 20:16 ` Eric Sunshine 2019-08-31 0:23 ` [PATCH v4] " Elijah Newren 2019-08-30 19:45 ` [PATCH v2] " Junio C Hamano 2019-08-30 19:48 ` Elijah Newren 2019-08-30 20:27 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git 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 read-only IMAP folder(s) and NNTP newsgroup(s).