| Message ID | 20231025102932.1202299-1-oswald.buddenhagen@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] git-rebase.txt: rewrite docu for fixup/squash (again) | expand |
On 2023-10-25 06:29, Oswald Buddenhagen wrote: > Create a clear top-down structure which makes it hopefully unambiguous > what happens when. > > The behavior in the presence of multiple "fixup -c" is somewhat > questionable, as arguably it would be better to complain about it rather > than letting the last instance win. But for the time being we document > the status quo, with a note that it is not guaranteed. Note that > actually changing it would require --autosquash eliding the superseded > uses. I do not think this kind of editorializing belongs in the commit's message, but this likely isn't the first commit message that expresses an opinion. > Also emphasize that the author info of the first commit is preserved > even in the presence of "fixup -c", as this diverges from "git commit > -c"'s behavior. New options matching the latter should be introduced for > completeness. > > Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> > > --- > v3: > - adjust to reality, and elaborate in the commit message why it's > arguably somewhat suboptimal > > i deliberated the 'command "pick"' word order swap suggested by marc, > but while it improves things locally, it somehow doesn't flow with the > "redundancy-reduced" last part of the sentence. > > v2: > - slight adjustments inspired by marc. however, i left most things > unchanged or even went in the opposite direction, because i assume the > readers to be sufficiently context-sensitive, and the objective is > merely to be not actively confusing. adding redundancy in the name of > clarity would just make the text stylistically inferior and arguably > harder to read. > > Cc: Junio C Hamano <gitster@pobox.com> > Cc: Phillip Wood <phillip.wood123@gmail.com> > Cc: Taylor Blau <me@ttaylorr.com> > Cc: Christian Couder <christian.couder@gmail.com> > Cc: Charvi Mendiratta <charvi077@gmail.com> > Cc: Marc Branchaud <marcnarc@xiplink.com> > --- > Documentation/git-rebase.txt | 30 ++++++++++++++++-------------- > 1 file changed, 16 insertions(+), 14 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index e7b39ad244..578d1d34a6 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -890,20 +890,22 @@ command "pick" with the command "reword". > To drop a commit, replace the command "pick" with "drop", or just > delete the matching line. > > -If you want to fold two or more commits into one, replace the command > -"pick" for the second and subsequent commits with "squash" or "fixup". > -If the commits had different authors, the folded commit will be > -attributed to the author of the first commit. The suggested commit > -message for the folded commit is the concatenation of the first > -commit's message with those identified by "squash" commands, omitting the > -messages of commits identified by "fixup" commands, unless "fixup -c" > -is used. In that case the suggested commit message is only the message > -of the "fixup -c" commit, and an editor is opened allowing you to edit > -the message. The contents (patch) of the "fixup -c" commit are still > -incorporated into the folded commit. If there is more than one "fixup -c" > -commit, the message from the final one is used. You can also use > -"fixup -C" to get the same behavior as "fixup -c" except without opening > -an editor. > +If you want to fold two or more commits into one (that is, to combine > +their contents/patches), replace the command "pick" for the second and > +subsequent commits with "squash" or "fixup". > +The commit message for the folded commit is the concatenation of the > +message of the first commit with those of commits identified by "squash" > +commands, omitting those of commits identified by "fixup" commands, > +unless "fixup -c" is used. In the latter case, the message is obtained > +only from the "fixup -c" commit (if multiple are present, the last one > +takes precedence, but this should not be relied upon). I like the overall phrasing here. But I think you should remove the "but this should not be relied upon" phrase. This reads as if Git's current behaviour is undefined, which most definitely is not true. Even changing this to something like "but this might change in the future" is unhelpful. Everything in Git is subject to change over a long-enough time span, so the same could be said about every aspect of Git. Until the behaviour actually changes, it's perfectly fine for people to use multiple "fixup -c" commands. There's no reason to scare them off of it. > +If the resulting commit message is a concatenation of multiple messages, > +an editor is opened allowing you to edit it. This is also the case for a > +message obtained via "fixup -c", while using "fixup -C" instead skips > +the editor; this is analogous to the behavior of `git commit`. > +The author information (including date/timestamp) always comes from > +the first commit; this is the case even if "fixup -c/-C" is used, > +contrary to what `git commit` does. This phrasing is much better. Thanks for putting up with my pedantry! M.
On Fri, Oct 27, 2023 at 09:14:42AM -0400, Marc Branchaud wrote: >On 2023-10-25 06:29, Oswald Buddenhagen wrote: >> The behavior in the presence of multiple "fixup -c" is somewhat >> questionable, as arguably it would be better to complain about it rather >> than letting the last instance win. But for the time being we document >> the status quo, with a note that it is not guaranteed. Note that >> actually changing it would require --autosquash eliding the superseded >> uses. > >I do not think this kind of editorializing belongs in the commit's >message, but this likely isn't the first commit message that expresses >an opinion. > commmit messages should elaborate alternatives considered, which includes ones which depend on changes that can be reasonably expected to possibly happen at some point. >But I think you should remove the "but this should not be relied upon" >phrase. This reads as if Git's current behaviour is undefined, which >most definitely is not true. > >Even changing this to something like "but this might change in the >future" is unhelpful. Everything in Git is subject to change over a >long-enough time span, so the same could be said about every aspect of Git. > >Until the behaviour actually changes, it's perfectly fine for people to >use multiple "fixup -c" commands. There's no reason to scare them off >of it. > things can't change overnight; the resistance even the most trivial behavior changes meet is enormous. so explicitly documenting long in advance that something is subject to change is basically the only way to get it changed at all. specifically for this feature, there is no reason at all to rely on this behavior when hand-editing the todo list, and occurrences most likely indicate a mistake, which is why i would prefer it to be rejected. regards
Marc Branchaud <marcnarc@xiplink.com> writes: > I do not think this kind of editorializing belongs in the commit's > message, but this likely isn't the first commit message that expresses > an opinion. Thanks for saying this. > I like the overall phrasing here. > > But I think you should remove the "but this should not be relied upon" > phrase. This reads as if Git's current behaviour is undefined, which > most definitely is not true. > > Even changing this to something like "but this might change in the > future" is unhelpful. Everything in Git is subject to change over a > long-enough time span, so the same could be said about every aspect of > Git. > > Until the behaviour actually changes, it's perfectly fine for people > to use multiple "fixup -c" commands. There's no reason to scare them > off of it. And that would simplify the description to make it easier to follow by readers who are *not* involved in the development process. > >> +If the resulting commit message is a concatenation of multiple messages, >> +an editor is opened allowing you to edit it. This is also the case for a >> +message obtained via "fixup -c", while using "fixup -C" instead skips >> +the editor; this is analogous to the behavior of `git commit`. >> +The author information (including date/timestamp) always comes from >> +the first commit; this is the case even if "fixup -c/-C" is used, >> +contrary to what `git commit` does. > > This phrasing is much better. > > Thanks for putting up with my pedantry! Thanks for a good review. I guess the patch is very near the finish line?
On 27/10/2023 14:14, Marc Branchaud wrote: >> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt >> index e7b39ad244..578d1d34a6 100644 >> --- a/Documentation/git-rebase.txt >> +++ b/Documentation/git-rebase.txt >> @@ -890,20 +890,22 @@ command "pick" with the command "reword". >> To drop a commit, replace the command "pick" with "drop", or just >> delete the matching line. >> -If you want to fold two or more commits into one, replace the command >> -"pick" for the second and subsequent commits with "squash" or "fixup". >> -If the commits had different authors, the folded commit will be >> -attributed to the author of the first commit. The suggested commit >> -message for the folded commit is the concatenation of the first >> -commit's message with those identified by "squash" commands, omitting >> the >> -messages of commits identified by "fixup" commands, unless "fixup -c" >> -is used. In that case the suggested commit message is only the message >> -of the "fixup -c" commit, and an editor is opened allowing you to edit >> -the message. The contents (patch) of the "fixup -c" commit are still >> -incorporated into the folded commit. If there is more than one "fixup >> -c" >> -commit, the message from the final one is used. You can also use >> -"fixup -C" to get the same behavior as "fixup -c" except without opening >> -an editor. >> +If you want to fold two or more commits into one (that is, to combine >> +their contents/patches), replace the command "pick" for the second and >> +subsequent commits with "squash" or "fixup". >> +The commit message for the folded commit is the concatenation of the >> +message of the first commit with those of commits identified by "squash" >> +commands, omitting those of commits identified by "fixup" commands, >> +unless "fixup -c" is used. In the latter case, the message is obtained >> +only from the "fixup -c" commit (if multiple are present, the last one >> +takes precedence, but this should not be relied upon). > > I like the overall phrasing here. > > But I think you should remove the "but this should not be relied upon" > phrase. This reads as if Git's current behaviour is undefined, which > most definitely is not true. I agree it would be better to remove that phrase, as you say it makes it sounds like the behaviour cannot be relied on. > Even changing this to something like "but this might change in the > future" is unhelpful. Everything in Git is subject to change over a > long-enough time span, so the same could be said about every aspect of Git. > > Until the behaviour actually changes, it's perfectly fine for people to > use multiple "fixup -c" commands. There's no reason to scare them off > of it. Indeed Best Wishes Phillip
On 2023-10-27 19:34, Junio C Hamano wrote: > > Thanks for a good review. I guess the patch is very near the finish > line? Yes. In my mind, all that's needed is to remove the part about "should not be relied upon". M.
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index e7b39ad244..578d1d34a6 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -890,20 +890,22 @@ command "pick" with the command "reword". To drop a commit, replace the command "pick" with "drop", or just delete the matching line. -If you want to fold two or more commits into one, replace the command -"pick" for the second and subsequent commits with "squash" or "fixup". -If the commits had different authors, the folded commit will be -attributed to the author of the first commit. The suggested commit -message for the folded commit is the concatenation of the first -commit's message with those identified by "squash" commands, omitting the -messages of commits identified by "fixup" commands, unless "fixup -c" -is used. In that case the suggested commit message is only the message -of the "fixup -c" commit, and an editor is opened allowing you to edit -the message. The contents (patch) of the "fixup -c" commit are still -incorporated into the folded commit. If there is more than one "fixup -c" -commit, the message from the final one is used. You can also use -"fixup -C" to get the same behavior as "fixup -c" except without opening -an editor. +If you want to fold two or more commits into one (that is, to combine +their contents/patches), replace the command "pick" for the second and +subsequent commits with "squash" or "fixup". +The commit message for the folded commit is the concatenation of the +message of the first commit with those of commits identified by "squash" +commands, omitting those of commits identified by "fixup" commands, +unless "fixup -c" is used. In the latter case, the message is obtained +only from the "fixup -c" commit (if multiple are present, the last one +takes precedence, but this should not be relied upon). +If the resulting commit message is a concatenation of multiple messages, +an editor is opened allowing you to edit it. This is also the case for a +message obtained via "fixup -c", while using "fixup -C" instead skips +the editor; this is analogous to the behavior of `git commit`. +The author information (including date/timestamp) always comes from +the first commit; this is the case even if "fixup -c/-C" is used, +contrary to what `git commit` does. `git rebase` will stop when "pick" has been replaced with "edit" or when a command fails due to merge errors. When you are done editing
Create a clear top-down structure which makes it hopefully unambiguous what happens when. The behavior in the presence of multiple "fixup -c" is somewhat questionable, as arguably it would be better to complain about it rather than letting the last instance win. But for the time being we document the status quo, with a note that it is not guaranteed. Note that actually changing it would require --autosquash eliding the superseded uses. Also emphasize that the author info of the first commit is preserved even in the presence of "fixup -c", as this diverges from "git commit -c"'s behavior. New options matching the latter should be introduced for completeness. Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de> --- v3: - adjust to reality, and elaborate in the commit message why it's arguably somewhat suboptimal i deliberated the 'command "pick"' word order swap suggested by marc, but while it improves things locally, it somehow doesn't flow with the "redundancy-reduced" last part of the sentence. v2: - slight adjustments inspired by marc. however, i left most things unchanged or even went in the opposite direction, because i assume the readers to be sufficiently context-sensitive, and the objective is merely to be not actively confusing. adding redundancy in the name of clarity would just make the text stylistically inferior and arguably harder to read. Cc: Junio C Hamano <gitster@pobox.com> Cc: Phillip Wood <phillip.wood123@gmail.com> Cc: Taylor Blau <me@ttaylorr.com> Cc: Christian Couder <christian.couder@gmail.com> Cc: Charvi Mendiratta <charvi077@gmail.com> Cc: Marc Branchaud <marcnarc@xiplink.com> --- Documentation/git-rebase.txt | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)