On Thu, Mar 2, 2023 at 2:37 AM Phillip Wood [off-list ref] wrote:

On 25/02/2023 18:03, Alex Henrie wrote:

quoted

+rebase.merges::
+     Whether and how to set the `--rebase-merges` option by default. Can
+     be `rebase-cousins`, `no-rebase-cousins`, or a boolean. Setting to
+     true is equivalent to `--rebase-merges` without an argument, setting to
+     `rebase-cousins` or `no-rebase-cousins` is equivalent to
+     `--rebase-merges` with that value as its argument, and setting to false
+     is equivalent to `--no-rebase-merges`. Passing `--rebase-merges` on the
+     command line without an argument overrides a `rebase.merges=false`
+     configuration but does not override other values of `rebase.merge`.

I'm still not clear why the commandline doesn't override the config in
all cases as is our usual practice. After all if the user has set
rebase.merges then they don't need to pass --rebase-merges unless they
want to override the config.

Given the current push to turn rebase-merges on by default, it seems
likely that rebase-cousins will also be turned on by default at some
point after that. There will be a warning about the default changing,
and we'll want to let users suppress that warning by setting
rebase.rebaseMerges=rebase-cousins. It would then be very confusing if
a --rebase-merges from some old alias continued to mean
--rebase-merges=no-rebase-cousins when the user expects it to start
behaving as though the default has already changed.

I will rephrase the documentation in v6 to make it more clear that the
absence of a specific value on the command line does not clobber a
specific value set in the configuration, as Glen suggested.

quoted

+test_expect_success '--rebase-merges=no-rebase-cousins overrides rebase.merges=rebase-cousins' '
+     test_config rebase.merges rebase-cousins &&
+     git checkout -b override-config-rebase-cousins main &&
+     git rebase --rebase-merges=no-rebase-cousins HEAD^ &&
+     test_cmp_graph HEAD^.. <<-\EOF
+     *   Merge the topic branch '\''onebranch'\''
+     |\
+     | * D
+     | * G
+     o | H
+     |/
+     o A
+     EOF
+'

I'm not sure this test adds much value, it is hard to see what kind of
regression would allow the others to pass but not this one.

I was worried that I or someone else would forget to explicitly set
rebase_cousins to 0 when no-rebase-cousins is given on the command
line, assuming that it is already 0 because that is the default. The
test makes me feel better, but I am happy to remove it if you still
think it's overkill.

quoted

+test_expect_success '--rebase-merges overrides rebase.merges=false' '
+     test_config rebase.merges false &&
+     git checkout -b override-config-merges-false E &&
+     before="$(git rev-parse --verify HEAD)" &&
+     test_tick &&
+     git rebase --rebase-merges C &&
+     test_cmp_rev HEAD $before

This test passes if the rebase does nothing, maybe pass --force and
check the graph?

The rebase is supposed to do nothing here. Checking that the commit
hash is the same is just a quick way to check that the entire graph is
the same. What more would be checked by checking the graph instead of
the hash?

Thanks for the feedback,

-Alex