On Wed, Apr 17, 2019 at 12:58:31AM -0700, Denton Liu wrote: > Thanks for the feedback. I couldn't find a tool that could selectively > fix indentation on patches so I went through and manually realigned the > parameter lists wherever the tools mangled the alignment. I guess this > also implies that one pair of (tired) human eyes has manually inspected > the machine-generated diff. > > Hopefully, patch 3/4 won't be as onerous to review as it was to write ;) Thanks. I looked over these manually and didn't find any problems (which isn't to say there aren't any, but now at least two sets of tired eyes looked at them :) ). This kind of cleanup is painful, but at least it should be done with after this, so I'm in favor of moving it forward. Two minor notes, though: > compat/mingw.c | 2 +- > compat/mingw.h | 6 +- > compat/nedmalloc/malloc.c.h | 6 +- > compat/obstack.h | 14 +- > compat/poll/poll.h | 2 +- > compat/regex/regex.h | 66 ++--- > compat/win32/pthread.h | 8 +- We sometimes avoid touching compat/ code for style issues because it's copied from elsewhere. And diverging from upstream is more evil than a pure style issue. So potentially we could drop these hunks (though I think maybe mingw is our own thing?). > contrib/coccinelle/noextern.cocci | 6 + I have mixed feelings on this cocci script. I'm happy to _see_ it, as it's important to show how the transformation was done. But for most of the other scripts, we expect programmers to introduce new cases that need converting, and we'd like to catch those automatically. Here I find it reasonably unlikely for a lot of "extern" to slip in, with the exception of some topics in flight. And these coccinelle scripts are kind of expensive to run. So I wonder if the tradeoff is worth it here (perhaps it is now, as we catch those topics in flight, it might be worth dropping this one in a few months). At any rate, thanks for doing all of this tedious work. :) -Peff
