Calvin Wan <calvinwan@xxxxxxxxxx> writes: > Remove the serial implementation of status inside of > is_submodule_modified since the parallel implementation of status with > one job accomplishes the same task. > > Combine parse_status_porcelain and parse_status_porcelain_strbuf since > the only other caller of parse_status_porcelain was in > is_submodule_modified I see that this is in direct response to Jonathan's earlier comment [1] that we should have only one implementation. Thanks, this is helpful. Definitely a step in the right direction. That said, I don't think this patch's position in the series makes sense. I would have expected a patch like this to come before 5/6. I.e. this series duplicates code in 5/6 and deletes it in 6/6 so that we only have one implementation for both serial and parallel submodule status. Instead, I would have expected we would refactor out the serial implementation, then use the refactored code for the parallel implementation. Not having duplicated code in 5/6 would shrink the line count a lot and make it easier to review. [1] https://lore.kernel.org/git/20221128210125.2751300-1-jonathantanmy@xxxxxxxxxx/