Calvin Wan <calvinwan@xxxxxxxxxx> writes: > A future patch requires the ability to parse the output of git > status --porcelain=2. Move parsing code from is_submodule_modified to > parse_status_porcelain. If my mental model is correct [1], i.e. that we are implementing a parallel version of is_submodule_modified(). I think we should be more explicit in this patch and the next, e.g.: In a later patch, we will implement a parallel version of is_submodule_modified(). Refactor its "git status --porcelain=2" parsing code so that we can reuse it both the parallel and non-parallel versions. If so, then this is pretty much doing the same thing as the next patch, so if the --color-moved diff isn't too bad, I think we can squash them, which will make the commit message easier to write too: In a later patch, we will implement a parallel version of is_submodule_modified(). Refactor its setup and parsing code so that we can reuse it both the parallel and non-parallel versions. - Setting up the subprocess is moved to prepare_status_porcelain() - XYZ is moved to verify_submodule_git_directory() - ABC is moved to parse_foobarbaz() Just an idea. I don't think squashing is necessarily better, but being explciit that we want a parallel version of is_submodule_modified() will make this easier to follow. [1] https://lore.kernel.org/git/kl6ljzzguqss.fsf@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx