| Message ID | 20230223053410.644503-1-alexhenrie24@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4,1/3] rebase: add documentation and test for --no-rebase-merges | expand |
Alex Henrie <alexhenrie24@gmail.com> writes: > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > --- No cover letter to summarize the changes? Comparing this round with the previous, the only two changes are that [1/3] lost one test "do not rebase merges unless asked to", and [2/3] now uses test_must_fail to mark a git command that is expected to stop. Looks good, but a rerolled patch series should not force reviewers to fetch and compare, but give a summary of changes when it is sent out, to save everybody's time. Preparing a summary before sending it out also helps the patch author, too, that all the intended changes are included in the new round. Thanks.
Hi Junio, On Thu, 23 Feb 2023, Junio C Hamano wrote: > Alex Henrie <alexhenrie24@gmail.com> writes: > > > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> > > --- > > No cover letter to summarize the changes? A range-diff would have been nice, too, as well as replying-to the previous iteration so that they're all within the same email thread. And if you want cover letters and range-diffs and correct In-Reply-To headers, I can think of a splendid way to encourage that: promote tools that do that. That's much better than to require contributors to know all the customs and conventions of the project and send mails in the exact desired format... Ciao, Johannes
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Thu, 23 Feb 2023, Junio C Hamano wrote: > >> Alex Henrie <alexhenrie24@gmail.com> writes: >> >> > Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> >> > --- >> >> No cover letter to summarize the changes? > > A range-diff would have been nice, too, as well as replying-to the > previous iteration so that they're all within the same email thread. Yes, especially the threading would help both those who missed previous iterations and those who reviewed them. Range-diff is often helpful when came with a good summary. Just like a patch alone without a good proposed log message is not a good way to help reviewers understand the issue the patch tries to solve, a cover with range-diff alone only shows what is different from the previous iteration, but does not say why the changes were made, so it is not a good substitute for an explanation by author's words. I may have suggested GGG if the author appeared a total beginner with the e-mail workflow (and especially if it were a single-patch topic), but Alex seems to be doing fine otherwise, and also GGG has its own learning curve (e.g. it is rare to see a good cover letter with per-iteration summaries, unless the author is one of those experienced list regulars), so I left it up to Alex to choose how to reach the desired end result. Thanks.
On Fri, Feb 24, 2023 at 6:58 AM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote: > On Thu, 23 Feb 2023, Junio C Hamano wrote: > > > No cover letter to summarize the changes? > A range-diff would have been nice, too, as well as replying-to the > previous iteration so that they're all within the same email thread. > > And if you want cover letters and range-diffs and correct In-Reply-To > headers, I can think of a splendid way to encourage that: promote tools > that do that. That's much better than to require contributors to know all > the customs and conventions of the project and send mails in the exact > desired format... It would also help if those expectations were mentioned in Documentation/SubmittingPatches. There's nothing in there about range-diff or In-Reply-To. I think I followed all of the customs of the Git mailing list for v5. Please let me know if I missed something. -Alex
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 9a295bcee4..c98784a0d2 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -529,13 +529,15 @@ See also INCOMPATIBLE OPTIONS below. -r:: --rebase-merges[=(rebase-cousins|no-rebase-cousins)]:: +--no-rebase-merges:: By default, a rebase will simply drop merge commits from the todo list, and put the rebased commits into a single, linear branch. With `--rebase-merges`, the rebase will instead try to preserve the branching structure within the commits that are to be rebased, by recreating the merge commits. Any resolved merge conflicts or manual amendments in these merge commits will have to be - resolved/re-applied manually. + resolved/re-applied manually. `--no-rebase-merges` can be used to + countermand a previous `--rebase-merges`. + By default, or when `no-rebase-cousins` was specified, commits which do not have `<upstream>` as direct ancestor will keep their original branch point, diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh index fa2a06c19f..d46d9545f2 100755 --- a/t/t3430-rebase-merges.sh +++ b/t/t3430-rebase-merges.sh @@ -250,6 +250,16 @@ test_expect_success 'with a branch tip that was cherry-picked already' ' EOF ' +test_expect_success '--no-rebase-merges countermands --rebase-merges' ' + git checkout -b no-rebase-merges E && + git rebase --rebase-merges --no-rebase-merges C && + test_cmp_graph C.. <<-\EOF + * B + * D + o C + EOF +' + test_expect_success 'do not rebase cousins unless asked for' ' git checkout -b cousins main && before="$(git rev-parse --verify HEAD)" &&
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> --- Documentation/git-rebase.txt | 4 +++- t/t3430-rebase-merges.sh | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-)