Paolo Bonzini <pbonz...@redhat.com> writes: > On 13/12/18 19:21, Peter Maydell wrote: >> On Thu, 13 Dec 2018 at 18:07, Paolo Bonzini <pbonz...@redhat.com> wrote: >>> On 13/12/18 19:01, Peter Maydell wrote: >>>> I sent a patch to do this a little while back: >>>> https://patchwork.kernel.org/patch/10561557/ >>>> >>>> It didn't get applied because Paolo disagreed with having >>>> our tools enforcing what our style guide says. >>> >>> I didn't disagree with that---I disagreed with having a single style in >>> the style guide, because unlike most other blatant violations of the >>> coding style (eg. braces), this one is pervasive in maintained code and >>> I don't want code that I maintain to mix two comment styles. >>> >>> So I proposed two alternatives: >>> >>> - someone fixes all the comment blocks which are "starred" but don't >>> have a lone "/*" at the beginning, and then we can commit that patch; >>> >>> - we allow "/* foo" on the first line, except for doc comments and for >>> the first line of the file (author/license block), and fix the style >>> guide accordingly. >> >> We came to a consensus on the comment style when we discussed >> the patch which updated CODING_STYLE. I'm not personally >> a fan of the result (I used to use "/* foo"), but what we have in the >> doc is what we achieved consensus for. I'm not going to reopen >> the "what should block comments look like" style debate. > > Sure, I don't want to do that either. I accept the result of the > discussion; I don't accept introducing a new warning that will cause > over 700 files to become inconsistent sooner or later.
By design, checkpatch.pl only checks *patches*. Existing code doesn't trigger warnings until it gets touched. And then it should arguably be made to conform to CODING_STYLE. So, what's the problem again? :) > Therefore, the > only way to enforce the result of the discussion is to change the > existing comments, I support cleaning up comment style wholesale[*]. > for example by having a script that maintainers can > use to change the existing comments in their files. Having each of us > come up with their own script or doing it by hand is probably not a good > use of everyone's time. Sharing tools is good. > Alternatively, fixing the style guide can also mean "explain why /* foo > is allowed by checkpatch even though it does not match the coding > style", without rehashing the discussion. > > (BTW it may actually be a good idea to fix _some_ instances of bad > coding style, in particular the space-tab sequences and the files where > there are maybe 2 or 3 tabs that ended up there by mistake. That's a > different topic). You've since posted patches for that. Thanks. >>>> Personally I think we should just commit my patch, and then >>>> we can stop having people manually pointing out where >>>> submitters' patches don't match CODING_STYLE. Concur. It has my R-by, modulo a commit message tweak. [*] Same for other style violations. Yes, it's churn, and yes, it'll mess up git-blame some, but I'm convinced the presence of numerous bad examples costs us more. CODING_STYLE was committed almost a decade ago. If we had cleaned up back then, the churn and the blame would be long forgotten, and we would've spared ourselves plenty of review cycles and quite a few style discussions. It's late, but never too late.
