On Fri, May 20 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > >> Follow-up 32ec72c3e1 (Merge branch 'ab/run-command', 2021-12-15) and >> rename the "env_array" member to simply "env" in the "struct >> child_process". >> >> Changes since v1: >> >> * Added a commit to remove the coccinelle rule after its application, >> as suggested by Junio in >> https://lore.kernel.org/git/xmqqzgjdkxon.fsf@gitster.g/ > > I actually think we do not even have to tentatively add one, only to > remove it at the end. > > Wouldn't a two-patch series whose > > - first patch does the real damage to the codebase, with the > coccinelle rule as part of the commit explanation in the proposed > log message (e.g. "the patch was mechanically prepared with the > help with this Coccinelle semantic patch"), and > > - the second patch does the non-mechanical "comments and names" > > work better than four patch series? A single patch that says "the > code was mechanically updated with this s-patch, with names and > comments that refer to env_array in the surrounding code manually > adjusted" would also work well. I can re-roll it if you insist, but I think it's much more useful to have rules added to contrib/coccinelle, even if they're subsequently removed within the same series. I've ofter paged through "git log -p -- contrib/coccinelle" to get inspiration from previous rules we used or see what's valid syntax. I think being able to do that consistently and not having to fish through log messages as well is more useful. It also makes it easier for someone who needs to modify their code to just "git checkout" and run "spatch" on the rule, without copy/pasting from a log message. But anyway, whatever you prefer. I see from the latest WC you're expecting a re-roll, maybe the above will convince you, and if not I can prepare a v3 with your suggested changes. Thanks.
