On Tue, Apr 28, 2020 at 04:36:28AM -0400, Denton Liu wrote: > In the codebase, there are many options which use OPTION_CALLBACK in a > plain ol' struct definition. However, we have the OPT_CALLBACK and > OPT_CALLBACK_F macros which are meant to abstract these plain struct > definitions away. These macros are useful as they semantically signal to > developers that these are just normal callback option with nothing fancy > happening. I think this is worth doing. It's a little easier to read, and sets a better example anyone copying the code. > Replace plain struct definitions of OPTION_CALLBACK with OPT_CALLBACK or > OPT_CALLBACK_F where applicable. The heavy lifting was done using the > following (disgusting) shell script: > [...] I'll admit I gave only a quick read through the results. I think between your script, the manual look-over for style, and the fact that the compiler would catch any minor syntactic screwups, I'm not likely to see any new errors. > I contemplated breaking this down into file-sized patches but I don't > think it really makes sense in this case since it's the same change > which is being made in each file and, imo, it wouldn't really ease > reviewer burden since the same number of changes are being reviewed. As a reviewer I much prefer one big patch like this, since these are all the same case: changing style X to style Y. I'd want patches broken out if there were other case (say, the parameters for some version needed fixed up as you were converting). But I don't see that here. -Peff
