On Mon, Mar 6, 2023 at 9:24 AM Phillip Wood [off-list ref] wrote:

On 05/03/2023 05:07, Alex Henrie wrote:

quoted

Suggestions on v5 not incorporated in v6:

Thanks for adding this, it is really helpful to see what did not change
as well as what did change. It is also helpful to briefly explain why
you disagree with the suggestions so others can understand why you
decided not to make these changes.

quoted

- Make --rebase-merges without an argument clobber the mode specified in
   rebase.rebaseMerges

I'm still confused as to why we want the config value to change the
behavior of --rebase-merges. Is it for "git pull --rebase=merges"? If
that's the case then I think a better approach is for pull to parse
rebase.merges and pass the appropriate argument to rebase. That way we
don't break the expectation that command line arguments override config
values.

quoted

- Remove the test for --rebase-merges=no-rebase-cousins overriding
   rebase.rebaseMerges=rebase-cousins
- In the tests, check the graph itself instead of checking that the
   graph has not changed by checking that the commit hash has not changed

I'm not sure what value the existing test has if it only checks that
HEAD is unchanged after the rebase. It could be unchanged because the
rebase fast-forwarded or because it did nothing.

Please have a look at the email I sent before sending v6:
https://lore.kernel.org/git/CAMMLpeRfD+81fsEtvKFvVepPpwYm0_-AD=QHMwhoc+LtiXpavw@mail.gmail.com/ (local)

In that email I tried to explain why I didn't incorporate your three
suggestions on v5. Please let me know if it still isn't clear.

-Alex