| Message ID | cover-v2-0.4-00000000000-20220520T072122Z-avarab@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | run-command.h: rename "env_array" to "env" | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.
On Fri, May 20 2022, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.
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/ * Minor commit rewording for 4/4 to adjust for that new 3/4. Ævar Arnfjörð Bjarmason (4): cocci: add a rename of "struct child_process"'s "env_array" to "env" run-command API: rename "env_array" to "env" cocci: remove env_array -> env migration run-command API users: use "env" not "env_array" in comments & names add-patch.c | 2 +- branch.c | 2 +- builtin/difftool.c | 6 ++--- builtin/receive-pack.c | 32 +++++++++++++-------------- builtin/stash.c | 16 +++++++------- builtin/submodule--helper.c | 44 ++++++++++++++++++------------------- builtin/worktree.c | 8 +++---- connect.c | 9 ++++---- connected.c | 2 +- daemon.c | 14 ++++++------ editor.c | 2 +- hook.c | 2 +- http-backend.c | 4 ++-- object-file.c | 2 +- pager.c | 4 ++-- promisor-remote.c | 2 +- run-command.c | 19 ++++++++-------- run-command.h | 14 ++++++------ sequencer.c | 18 +++++++-------- submodule.c | 38 ++++++++++++++++---------------- submodule.h | 4 ++-- t/helper/test-run-command.c | 2 +- trailer.c | 2 +- transport-helper.c | 2 +- wt-status.c | 2 +- 25 files changed, 127 insertions(+), 125 deletions(-) Range-diff against v1: 1: b943ed5d5b7 = 1: e3f33fce566 cocci: add a rename of "struct child_process"'s "env_array" to "env" 2: 51063a31b16 = 2: 5aeb6ec9117 run-command API: rename "env_array" to "env" -: ----------- > 3: ca09da570ef cocci: remove env_array -> env migration 3: 8af76f39b9f ! 4: 5419f839c74 run-command API users: use "env" not "env_array" in comments & names @@ Metadata ## Commit message ## run-command API users: use "env" not "env_array" in comments & names - Follow-up on the preceding commit which changed all references to the + Follow-up on a preceding commit which changed all references to the "env_array" when referring to the "struct child_process" member. These changes are all unnecessary for the compiler, but help the code's human readers.