Re: [PATCH v4 0/9] rebase: fix several code/testing/documentation issues around flag incompatibilities

From: Derrick Stolee <derrickstolee@github.com>
Date: Jan 23, 2023, 15:56 UTC
Message-ID: <1d9748df-4f54-908d-75cf-49ff1d154fcb@github.com>
In-Reply-To: <pull.1466.v4.git.1674367961.gitgitgadget@gmail.com>

On 1/22/2023 1:12 AM, Elijah Newren via GitGitGadget wrote:

Show 10 quoted lines

> We had a report about --update-refs being ignored when --whitespace=fix was
> passed, confusing an end user. These were already marked as incompatible in
> the manual, but the code didn't check for the incompatibility and provide an
> error to the user.
> 
> Folks brought up other flags tangentially when reviewing an earlier round of
> this series, and I found we had more that were missing checks, and that we
> were missing some testcases, and that the documentation was wrong on some of
> the relevant options. So, I ended up getting lots of little fixes to
> straighten these all out.

Wow, this really expanded since I last looked at it. Thanks for taking on all
of that extra work! (That was not my intention when recommending that you take
over the fix.)

Show 6 quoted lines

> Changes since v3:
> 
>  * Corrected the code surrounding --[no-]reapply-cherry-picks, and extended
>    the testcases (Thanks to Phillip for pointing out my error)
>  * I went ahead and implemented the better error message when the merge
>    backend is triggered solely via config options.

I really appreciate this extra attention to detail. I'm also really happy with how you implemented it, using different variables to signal how the option was specified (and using -1 for "unset" in both cases).

While I had not been following version 2 or 3, I read this version in its entirety and everything looked good to me. These improvements to our docs, tests, and implementation will be felt by users.

Thanks! -Stolee