On Thu, Mar 01, 2018 at 12:43:22PM +0200, Jani Nikula wrote: > On Thu, 01 Mar 2018, Arkadiusz Hiler <arkadiusz.hiler@xxxxxxxxx> wrote: > > Since not so long ago our CI is running and reporting sparse and > > checkpatch. Sparse is doing just fine but I had to disable checkpatch > > for the time being - too much "false" positives causing people to > > complain. It's simply confusing to see one thing in the code, and > > fitting your change in only to get a report that it's wrong. > > > > We are explicitly going against couple of the recommendations it tries > > to enforce, e.g. not using BIT macro, splitting quoted strings: > > https://lists.freedesktop.org/archives/intel-gfx/2018-February/156613.html > > > > IMHO we should make a couple of decisions here: > > 1. Do we really want to use the checkpatch / have CI reports? > > I think yes, for the benefit of both patch authors and reviewers. For > the most part, we do want to encourage uniform style. > > > 2. Which of the checkpatch checks we want to disabled for i915? > > One low hanging fruit is to ignore the CHECK messages, or drop the > --strict option to checkpatch.pl in CI, although I think some of them > are nice. IMO the strict vs. not split is totally arbitrary. Some really obviously correct warning are only enabled with strict, whereas even w/o strict you get some warnings that are just plain silly. Thus I think strict does more good than harm. > > > 3. How strongly do we want to enforce the rest? > > That's a tough one. With code movement, you want the code to remain the > same instead of changing at the same time. And some of the warnings are > subjective. For example, I'd prefer to stick with the 80 column rule but > only when it makes sense. ;) > > Another example, I would like to move towards kernel types over uint8_t > and friends. However, when you have code surrounded by uint8_t and > friends, it's often better to stick with the style around you. > > > 4. Do we want to change what's already in the tree, for compliance? > > No. I don't think we should encourage mindless checkpatch fixes. > > Does checkpatch support disabling checks or do you have to filter them > out from the output? > > BR, > Jani. > > > > > > Recent-ish drm-tip, vanilla checkpatch on i915 reports: > > total: 399 errors, 3573 warnings, 209374 lines checked > > -- > Jani Nikula, Intel Open Source Technology Center > _______________________________________________ > dim-tools mailing list > dim-tools@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dim-tools -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
