All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org, dim-tools@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules
Date: Tue, 13 Mar 2018 12:55:36 +0100	[thread overview]
Message-ID: <20180313115536.GD4788@phenom.ffwll.local> (raw)
In-Reply-To: <20180313113010.13078-3-jani.nikula@intel.com>

On Tue, Mar 13, 2018 at 01:30:10PM +0200, Jani Nikula wrote:
> Set max line length to 100. I don't want to silence the LONG_LINE
> warning altogether, and I'd still prefer to keep lines under 80
> characters, but I also don't want to see all the noise, and nor do I
> want to see silly code trying to arbitrarily squeeze under 80 when it
> doesn't make sense. 100 is a nice arbitrary round number... I hope
> review catches silly stuff regardless. Fingers crossed.
> 
> BIT_MACRO. We have (1 << N) all over the place. I hope to switch to
> BIT() macro eventually, but this documents current use.
> 
> PREFER_KERNEL_TYPES. We also have uint(8|16|32|64)_t all over the
> place. I also hope to move towards kernel types, but this documents
> current use.
> 
> SPLIT_STRING, LONG_LINE_STRING. Don't nag about strings split to many
> lines, but also don't nag about strings not split.
> 
> There's plenty more that could be tweaked, but let's start with
> something to improve the S/N ratio of the automated CI checkpatch
> reports. Now that we have --show-types included in the output, we can
> more easily discuss the ignores on a case-by-case basis.
> 
> Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  dim | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/dim b/dim
> index 4ba1c7ff490a..9fa6d9cd855b 100755
> --- a/dim
> +++ b/dim
> @@ -1390,7 +1390,7 @@ function checkpatch_commit
>  			profile_options=""
>  			;;
>  		drm-intel)
> -			profile_options=""
> +			profile_options="--max-line-length=100 --ignore=BIT_MACRO,PREFER_KERNEL_TYPES,SPLIT_STRING,LONG_LINE_STRING"

I've scrolled a bit through checkpatch complaines with this, and I think
it looks a lot more reasonable. There's also a huge pile of stuff that we
should probably have fixed when the patches landed, so CI'ing this looks
good.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Aside: Should we encourage checkpatch patches to clean this up, with the
note that they must use the drm-intel profile and the caveat that we might
want to add more stuff to our ignore list instead of taking the patches?
-Daniel

>  			;;
>  		*)
>  			echoerr "Unknown checkpatch profile $profile"
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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

  reply	other threads:[~2018-03-13 11:55 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-13 11:30 [DIM PATCH 1/3] dim: cleanup checkpatch_commit Jani Nikula
2018-03-13 11:30 ` [DIM PATCH 2/3] dim: add checkpatch profiles to allow different checkpatch options Jani Nikula
2018-03-13 11:48   ` Daniel Vetter
2018-03-13 15:07     ` Jani Nikula
2018-03-14  6:29       ` Daniel Vetter
2018-03-14  8:57         ` Jani Nikula
2018-03-13 11:30 ` [DIM PATCH 3/3] dim: loosen some drm-intel checkpatch rules Jani Nikula
2018-03-13 11:55   ` Daniel Vetter [this message]
2018-03-14  9:03     ` 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=20180313115536.GD4788@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=dim-tools@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.