Attention is currently required from: Julius Werner. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50247 )
Change subject: Documentation: Codify some guidelines for headers and chain-including ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: Sorry for the useless comments yesterday. I was indirectly pointed here after asking for the reason of such an odd "cleanup" patch ;) When I read the text then and saw that it recently changed without notification, it felt like being stabbed in the back by it, really, really hard. After 9 years one might think they know the project, then, suddenly things change.
I've read the full text here now and see your good intentions. I think it's mostly the wording that is too strong: "should always" encourages people to "cleanup" patches. I guess, before adding more rules that can lead to such patches, we have to make it clear that coding style guidelines are not to be applied bluntly.
If I would change something in this document right now, it would be adding something like this:
Style decisions of the original author should be honored (unless they made a mess of it). Patches that apply this coding style bluntly to existing code shall be ignored.
It feels like 90% of the cleanup patches I see are noise, even some that are encouraged by linters/checkpatch.
Is this just about the "alphabetical order" part? I think most people prefer that and maybe some really hate it and I personally don't care enough to argue, so I was hoping phrasing it as "preferably" leaves everyone enough wiggle room to be happy. If that really bothers you then I can go delete that part again and we can go back to arguing about this on each CL individually. I really don't care enough to start a fight over it.
Only a little; later...
If this is actually about the chain-including part, can you please clarify what exactly you don't like about it so we can try to fix it forward? Because this is (mostly) how our existing code looks right now, and not having any rules at all for it means everyone keeps trying to upload "cleanup" patches to pull things in opposite directions which is just a huge waste of everyone's time.
No, I agree to that part. However, I would argue that it doesn't belong to coding style. In a perfect world, the order of includes wouldn't matter. But we don't live in one, and we can't control all headers we include. Thus, sooner or later, we'll stumble over exceptions and have to consider semantics.
There is something I read among your lines that concerns me, but I've seen it as well: there are patches that must be stopped. IMHO, that should be addressed more globally. I'm not sure how. More obligations for submitters come to mind, but I can already hear the argument that we lack the resources...