* Rafael J. Wysocki <rjw@xxxxxxx> wrote: > > > +resume_devices: > > > + resume_device_irqs(); > > > > Small style nit: labels should start with a space character. > > I.e. it should be: > > I know, but the second label in there starts without a space > character and IMO keeping a uniform coding style i a single > file is more important than trying to adjust it to a broader > set of rules FWIW. [...] Even though it's just a very small and insignificant detail (nowhere described in the CodingStyle), barely worth the mention (and i already regret having brought it up at all), what you say is wrong on a conceptual level and that alarms me a bit ;-) It is exactly these kinds of "my code, my style!" world view that results in a crappy overall kernel style. For a single file to look consistent is just the first (and required) step, what matters even more is for files to have similar coding patterns, to make the style as helpful to the general kernel developer/reviewer/bug-fixer/maintainer as possible. "code with a helpful style" here means two things: 1) it should understand and adhere to basic style principles. This is just an (often arbitrary) subset of the infinite set of reasonable style guides. The most common-sense ones are written down in Documentation/CodingStyle. There's a lot of leeway, as long as the basic principle of "be helpful" is understood and followed. 2) it should carry meta information outside of the language syntax and it should build expectations about a code's purpose and general structure. That is essential so that we can find bugs during review. If each file has a slightly different style to express labels then that means we insert extra entropy and degrades and obfuscates the true meat of the code and hurts the overall reviewability of the code. In practical terms: i noticed that weird label - otherwise i would not have commented on it. I noticed it because it had the pattern of a comment block (most comment blocks start with capital letters, and for that good reason). It was completely unnecessary for me to notice that label - it carries no information about the patch itself. Ergo, it would be better in the long run if code does not raise unnecessary mental exceptions. We have a limited set of exceptions we are able to handle during review, lets make sure we use them sparingly. Sure, there will always be borderline cases where we'll have to agree to disagree, even if we agree about the general principle. But this is not one of those cases - having a "Suspend:" capitalized label is not something you added to enhance the basic coding style - it is something very uncommon and self-serving which you added in _spite_ of the general principles i believe. It has no other message beyond "I do this because i can!". I.e. it is not helpful at all. When it comes to coding style the kernel is not a democracy at all. > [...] I also think that coding style changes shouldn't be > mixed with functional changes as far as reasonably possible. Sure, you got that drive-by review for free, by virtue of context diffs ;-) Ingo _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm
