| Message ID | 20190124131104.GA24017@sigill.intra.peff.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | drop some unused parameters | expand |
On 1/24/2019 8:11 AM, Jeff King wrote:
> 2. Patches that drop unused parameters (i.e., code cleanup).
I've read the patches and they do exactly this, along with explanations
of why they don't fit in the first category.
Thanks,
-Stolee
Jeff King <peff@peff.net> writes: > The patches themselves are pretty much independent from each other. I > based these on master. They're cleanup which _could_ go to maint, and I > suspect they'd apply there, too (most of these are pretty old), but > again, I don't think there's a particular urgency to this. Thanks for being considerate. One potential benefit that we might gain by applying these clean-ups also to 'maint' is that it would allow us to keep 'master' and 'maint' more in sync, so it would make it more likely for unrelated bugfixes we may have in the future in the vicinity to also apply cleanly to both. By definition, the variables removed are not used, so it would make an interesting situation if such a future bugfix needs to look at them (sort of turning the changes in this series into category 1 "we should have been looking at them, but we didn't"), but at that point, we'd be resurrecting these variables we drop here, so it won't be really hurting us. And if such a bugfix does not need to look at them, then textual context that might be caused by not applying these to 'maint' but keeping them only to 'master' would not be a big deal, either. So I can go either way, but I am tempted to leave them outside 'maint' --- at least that is my current thinking. Thanks. > > [1/8]: match-trees: drop unused path parameter from score functions > [2/8]: apply: drop unused "def" parameter from find_name_gnu() > [3/8]: create_bundle(): drop unused "header" parameter > [4/8]: column: drop unused "opts" parameter in item_length() > [5/8]: show_date_relative(): drop unused "tz" parameter > [6/8]: config: drop unused parameter from maybe_remove_section() > [7/8]: convert: drop len parameter from conversion checks > [8/8]: convert: drop path parameter from actual conversion functions > > apply.c | 5 ++--- > builtin/bundle.c | 3 +-- > bundle.c | 4 ++-- > bundle.h | 4 ++-- > cache.h | 2 +- > column.c | 4 ++-- > config.c | 3 +-- > convert.c | 28 ++++++++++++++-------------- > date.c | 8 ++++---- > match-trees.c | 16 +++++++--------- > t/helper/test-date.c | 2 +- > 11 files changed, 37 insertions(+), 42 deletions(-)
On Thu, Jan 24, 2019 at 08:11:05AM -0500, Jeff King wrote: > I've mentioned before that I have a series which compiles cleanly with > -Wunused-parameters. I split this work roughly into three groups: > > 1. Patches that fix bugs (i.e., we should have been using the > parameter but didn't). > > 2. Patches that drop unused parameters (i.e., code cleanup). > > 3. Patches that annotate undroppable parameters (e.g., ones that are > present due to a callback interface). > > All of the patches from group 1 have been merged already. So this series > starts us off on group 2. There are about 50 patches in that group. > Given that none of them are urgent, I plan to feed them in batches to > avoid overwhelming reviewers. I'm also ordering them to avoid conflicts > with other topics in flight (this batch has no conflicts with 'next', > and only one minor textual conflict with 'pu'). All of these patches seemed straightforward. I did see a few where there may have originally been some consistency benefit to keeping parameters (always passing file name to *_to_worktree, for example), but I'm fine dropping them if it means we can get better development help from the compiler. I am also, for the record, in favor of ignoring the effects of relativity for the purposes of Git. :)
I've mentioned before that I have a series which compiles cleanly with -Wunused-parameters. I split this work roughly into three groups: 1. Patches that fix bugs (i.e., we should have been using the parameter but didn't). 2. Patches that drop unused parameters (i.e., code cleanup). 3. Patches that annotate undroppable parameters (e.g., ones that are present due to a callback interface). All of the patches from group 1 have been merged already. So this series starts us off on group 2. There are about 50 patches in that group. Given that none of them are urgent, I plan to feed them in batches to avoid overwhelming reviewers. I'm also ordering them to avoid conflicts with other topics in flight (this batch has no conflicts with 'next', and only one minor textual conflict with 'pu'). The patches themselves are pretty much independent from each other. I based these on master. They're cleanup which _could_ go to maint, and I suspect they'd apply there, too (most of these are pretty old), but again, I don't think there's a particular urgency to this. [1/8]: match-trees: drop unused path parameter from score functions [2/8]: apply: drop unused "def" parameter from find_name_gnu() [3/8]: create_bundle(): drop unused "header" parameter [4/8]: column: drop unused "opts" parameter in item_length() [5/8]: show_date_relative(): drop unused "tz" parameter [6/8]: config: drop unused parameter from maybe_remove_section() [7/8]: convert: drop len parameter from conversion checks [8/8]: convert: drop path parameter from actual conversion functions apply.c | 5 ++--- builtin/bundle.c | 3 +-- bundle.c | 4 ++-- bundle.h | 4 ++-- cache.h | 2 +- column.c | 4 ++-- config.c | 3 +-- convert.c | 28 ++++++++++++++-------------- date.c | 8 ++++---- match-trees.c | 16 +++++++--------- t/helper/test-date.c | 2 +- 11 files changed, 37 insertions(+), 42 deletions(-)