Re: [PATCH v2 4/7] merge-ort: clearer propagation of failure-to-function from merge_submodule

Derrick Stolee
2024-06-28T02:12:10Z

On 6/18/24 11:00 PM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
>
> The 'clean' member variable is somewhat of a tri-state (1 = clean, 0 =
> conflicted, -1 = failure-to-determine), but we often like to think of
> it as binary (ignoring the possibility of a negative value) and use
> constructs like '!clean' to reflect this. However, these constructs
> can make codepaths more difficult to understand, unless we handle the
> negative case early and return pre-emptively; do that in
> handle_content_merge() to make the code a bit easier to read.

This patch is correct and valuable.

Would it be valuable to go a bit further and turn 'clean' into
an enum that reflects these states? Perhaps that would prevent
future changes from slipping into this mistake.

Thanks,
-Stolee