On Fri, 19 Aug 2022 at 16:28, Justin Pryzby <pryzby@telsasoft.com> wrote:
> Let me know what I can do when it's time for round two.
I pushed the modified 0001-0008 patches earlier today and also the one
I wrote to fixup the 36 warnings about "expected" being shadowed.
I looked through a bunch of your remaining patches and was a bit
unexcited to see many more renaming such as:
- List *querytree_list;
+ List *this_querytree_list;
I don't think this sort of thing is an improvement.
However, one category of these changes that I do like are the ones
where we can move the variable into an inner scope. Out of your
renaming 0009-0026 patches, these are:
0013
0014
0017
0018
I feel like having the variable in scope for the minimal amount of
time makes the code cleaner and I feel like these are good next steps
because:
a) no variable needs to be renamed
b) any backpatching issues is more likely to lead to compilation
failure rather than using the wrong variable.
Likely 0016 is a subcategory of the above as if you modified that
patch to follow this rule then you'd have to declare the variable a
few times. I think that category is less interesting and we can maybe
consider those after we're done with the more simple ones.
Do you want to submit a series of patches that fixes all of the
remaining warnings that are in this category? Once these are done we
can consider the best ways to fix and if we want to fix any of the
remaining ones.
Feel free to gzip the patches up if the number is large.
David