On Fri, 14 Feb 2025 at 11:35, Elijah Newren <newren@xxxxxxxxx> wrote: > > > > Did you run any tests? I'm not sure you maintained correctness here. > > > > I didn't run any tests -- I wanted to, but I wasn’t sure how to do it > > for this change. Since you suggested dropping this patch from the > > series, I’ll do that. But for similar changes in the future, how should I go > > about testing them? > > As per Documentation/CodingGuidelines: "After any code change, make > sure that the entire test suite passes." You can do that by running: > cd t && make > (You probably want to also run that before making any changes, just to > verify that they all pass for you. Then, if any test fails after you > make changes, you know it's because of your changes rather than > because you missed something in building or setting up the tests.) > > > And although it doesn't matter since we're dropping this patch, the > issue I noticed was that if there were, say, three unmerged entries > with the same path, the original code would create one entry in the > string list and modify it 3 times (each with a different ce_stage(ce). > Your modification would create three different entries (each with only > information from one stage) and drop two of them, meaning we no longer > have a single string_list_item that contains information from all 3 > unmerged entries for the same path. I'm pretty sure running the > existing tests would catch that kind of bug, which is what raised the > question. That's the thing -- I did run make in the t/ directory, and it passed. I was just wondering if there's any other way to test this in isolation, in case I want to verify such changes more directly in the future. Thanks for the clarification! Meet