| Message ID | pull.803.git.1607011187.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | merge-ort: some groundwork for further implementation | expand |
On 12/3/2020 10:59 AM, Elijah Newren via GitGitGadget wrote: > This series is based on en/merge-ort-impl. This series sets up three future > patch series (to add recursive merges, three-way content merging, and rename > detection) for the merge-ort implementation, and allows the future series to > be submitted, reviewed, and merged in any order. Since those three things > actually do have some minor dependencies between them, this preparatory > series is needed to make a few small changes to set things up to allow them > to be submitted independently. > > The first six patches are trivial. They should be easy to review, and as a > bonus you get to find how I mess up even the trivial stuff. ;-) The final > patch is more substantive and represents one of the big changes between > merge-recursive and merge-ort -- namely, how notice/warning/conflict > messages are reported to the user (I possibly should have included it in > merge-ort-impl, but that series seemed so long already...). > > Elijah Newren (7): > merge-ort: add a few includes > merge-ort: add a clear_internal_opts helper > merge-ort: add a path_conflict field to merge_options_internal > merge-ort: add a paths_to_free field to merge_options_internal > merge-ort: add function grouping comments > merge-ort: add die-not-implemented stub handle_content_merge() > function > merge-ort: add modify/delete handling and delayed output processing Coming back to say that I finished reading PATCH 7 and this series looks good overall. Tough to be confident in it when the implementation isn't connected to tests, but the patches do a good job of describing the isolated changes. If there _are_ problems, it will be easy to identify the reasoning behind the code using log/blame. Thanks, -Stolee
On Fri, Dec 4, 2020 at 9:26 AM Derrick Stolee <stolee@gmail.com> wrote: > > On 12/3/2020 10:59 AM, Elijah Newren via GitGitGadget wrote: > > This series is based on en/merge-ort-impl. This series sets up three future > > patch series (to add recursive merges, three-way content merging, and rename > > detection) for the merge-ort implementation, and allows the future series to > > be submitted, reviewed, and merged in any order. Since those three things > > actually do have some minor dependencies between them, this preparatory > > series is needed to make a few small changes to set things up to allow them > > to be submitted independently. > > > > The first six patches are trivial. They should be easy to review, and as a > > bonus you get to find how I mess up even the trivial stuff. ;-) The final > > patch is more substantive and represents one of the big changes between > > merge-recursive and merge-ort -- namely, how notice/warning/conflict > > messages are reported to the user (I possibly should have included it in > > merge-ort-impl, but that series seemed so long already...). > > > > Elijah Newren (7): > > merge-ort: add a few includes > > merge-ort: add a clear_internal_opts helper > > merge-ort: add a path_conflict field to merge_options_internal > > merge-ort: add a paths_to_free field to merge_options_internal > > merge-ort: add function grouping comments > > merge-ort: add die-not-implemented stub handle_content_merge() > > function > > merge-ort: add modify/delete handling and delayed output processing > > Coming back to say that I finished reading PATCH 7 and this series > looks good overall. Tough to be confident in it when the implementation > isn't connected to tests, but the patches do a good job of describing > the isolated changes. If there _are_ problems, it will be easy to > identify the reasoning behind the code using log/blame. Doh, I should have mentioned that this series drops the number of failures in the testsuite when run under GIT_TEST_MERGE_ALGORITHM=ort from 1453 to 1448. (Due to the final patch adding handling for modify/delete conflicts.) It feels almost like an oversight since the number only dropped by 5 and the focus was on setting up the next three series which will drop it much more. The next three, which I'm planning to submit soon, will collectively drop the number of failures by 1388 down to just 60. Anyway, thanks for looking it over!
This series is based on en/merge-ort-impl. This series sets up three future patch series (to add recursive merges, three-way content merging, and rename detection) for the merge-ort implementation, and allows the future series to be submitted, reviewed, and merged in any order. Since those three things actually do have some minor dependencies between them, this preparatory series is needed to make a few small changes to set things up to allow them to be submitted independently. The first six patches are trivial. They should be easy to review, and as a bonus you get to find how I mess up even the trivial stuff. ;-) The final patch is more substantive and represents one of the big changes between merge-recursive and merge-ort -- namely, how notice/warning/conflict messages are reported to the user (I possibly should have included it in merge-ort-impl, but that series seemed so long already...). Elijah Newren (7): merge-ort: add a few includes merge-ort: add a clear_internal_opts helper merge-ort: add a path_conflict field to merge_options_internal merge-ort: add a paths_to_free field to merge_options_internal merge-ort: add function grouping comments merge-ort: add die-not-implemented stub handle_content_merge() function merge-ort: add modify/delete handling and delayed output processing merge-ort.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 191 insertions(+), 19 deletions(-) base-commit: 00de8a7763e29fb8a034030afbd0fbfc4c818e07 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-803%2Fnewren%2Fort-common-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-803/newren/ort-common-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/803