From: Daniel Vetter <daniel@ffwll.ch>
To: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: dim-tools@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: i915 vs checkpatch
Date: Mon, 5 Mar 2018 09:14:27 +0100 [thread overview]
Message-ID: <20180305081427.GF6419@phenom.ffwll.local> (raw)
In-Reply-To: <20180301094706.GA27356@ahiler-desk1.ger.corp.intel.com>
On Thu, Mar 01, 2018 at 11:47:06AM +0200, Arkadiusz Hiler wrote:
> Hey all,
>
> 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?
> 2. Which of the checkpatch checks we want to disabled for i915?
> 3. How strongly do we want to enforce the rest?
> 4. Do we want to change what's already in the tree, for compliance?
>
> Recent-ish drm-tip, vanilla checkpatch on i915 reports:
> total: 399 errors, 3573 warnings, 209374 lines checked
I'd recommend not making checkpatch ever fail CI, but at most warning.
Plus silence the ones we obviously think are silly (or currently
inconsistent in our code).
The reason is that checkpatch isn't really maintained by a consensus
approach, but it's mostly just Joe Perches doing what he feels like. It's
better again, but in past kernel releases it has become opionated about
pointless bikesheds to the point I simply had to ignore most of it. It'd
be great if we have an authoritative answer to all code layout questions,
but in reality we don't.
I think the ingore list is probably best kept within maintainer-tools
itself, that way we at least have visibility into it from committers.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-03-05 8:14 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-01 9:47 i915 vs checkpatch Arkadiusz Hiler
2018-03-01 10:43 ` Jani Nikula
2018-03-01 11:21 ` Ville Syrjälä
2018-03-02 16:07 ` Jani Nikula
2018-03-01 16:13 ` Jani Nikula
2018-03-01 18:00 ` Rodrigo Vivi
2018-03-02 9:37 ` Joonas Lahtinen
2018-03-02 10:07 ` Jani Nikula
2018-03-01 23:17 ` Chris Wilson
2018-03-05 12:55 ` Arkadiusz Hiler
2018-03-05 8:14 ` Daniel Vetter [this message]
2018-03-05 11:10 ` Jani Nikula
2018-03-05 12:44 ` Arkadiusz Hiler
2018-03-13 11:38 ` Jani Nikula
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180305081427.GF6419@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=arkadiusz.hiler@intel.com \
--cc=dim-tools@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=rodrigo.vivi@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox