Re: [PATCH v2 04/13] fast-rebase: write conflict state to working tree, index, and HEAD
flat view
From: Derrick Stolee <hidden>
Date: 2021-05-18 13:54:42
On 5/17/2021 11:42 PM, Elijah Newren wrote:
On Mon, May 17, 2021 at 6:32 AM Derrick Stolee [off-list ref] wrote:quoted
On 5/3/21 10:12 PM, Elijah Newren via GitGitGadget wrote:quoted
From: Elijah Newren <redacted> Previously, when fast-rebase hit a conflict, it simply aborted and left HEAD, the index, and the working tree where they were before the operation started. While fast-rebase does not support restarting from a conflicted state, write the conflicted state out anyway as it gives us a way to see what the conflicts are and write tests that check for them. This will be important in the upcoming commits, because sequencer.c is only superficially integrated with merge-ort.c; in particular, it calls merge_switch_to_result() after EACH merge instead of only calling it at the end of all the sequence of merges (or when a conflict is hit). This not only causes needless updates to the working copy and index, but also causes all intermediate data to be freed and tossed, preventing caching information from one merge to the next. However, integrating sequencer.c more deeply with merge-ort.c is a big task, and making this small extension to fast-rebase.c provides us with a simple way to test the edge and corner cases that we want to make sure continue working.This seems a noble goal. I'm worried about the ramifications of such a change, specifically about the state an automated process would be in after hitting a conflict.Why would an automated process be using test-helper code? Note that this is code from t/helper/test-fast-rebase.c.
...
quoted
So perhaps this could use a test case to demonstrate the change in behavior? Such a test would want to be created before this commit, then the behavior change is provided as an edit to the test in this commit. But maybe I'm misunderstanding the change here and such a test is inappropriate.If this weren't code under t/helper/, I'd agree whole-heartedly with the suggestion.
You're right. Ignore me. -Stolee