linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Colin King <colin.king@canonical.com>,
	Tom Cooksey <tom.cooksey@arm.com>, Eric Anholt <eric@anholt.net>,
	David Airlie <airlied@linux.ie>,
	dri-devel@lists.freedesktop.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH][drm-next] drm/pl111: make structure pl111_display_funcs static
Date: Fri, 19 May 2017 22:47:59 +0300	[thread overview]
Message-ID: <20170519194759.gfiefx6xwrnrjn3x@mwanda> (raw)
In-Reply-To: <874lwhoxyk.fsf@intel.com>

On Fri, May 19, 2017 at 03:03:31PM +0300, Jani Nikula wrote:
> On Fri, 19 May 2017, Colin King <colin.king@canonical.com> wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > structure pl111_display_funcs can be made static as it does not need to be
> > in global scope.  Fixes sparse warning:
> >
> > "warning: symbol 'pl111_display_funcs' was not declared. Should it
> > be static?"
> >
> > Fixes: bed41005e6174d ("drm/pl111: Initial drm/kms driver for pl111")
> 
> The patch looks good and I appreciate what you're doing, but I question
> the usefulness of adding Fixes: tags for trivial stuff like this. I'd
> prefer Fixes: was reserved for actual fixes that should be backported to
> any kernels that have the commit being fixed.
> 
> The same applies to many other patches you've sent recently.
> 

The Fixes tag is so so useful for everything.  It should be included
in every bugfix.  (I am the inventor of the Fixes tag).

I told Colin to include the Fixes tag on everything.  My review process
is partly "How was this bug introduced?  How can we prevent it from
happening again?  Who was the original author and have they reviewed the
proposed fix?"  So I end up looking up the original commit anyway.  It
helps me a lot to have the Fixes tag there.

The Fixes tag is obviously useful for the stable people as well, but
that wasn't really the point.

regards,
dan carpenter

  parent reply	other threads:[~2017-05-19 19:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-19 11:02 [PATCH][drm-next] drm/pl111: make structure pl111_display_funcs static Colin King
2017-05-19 12:03 ` Jani Nikula
2017-05-19 18:19   ` Eric Anholt
2017-05-19 19:40     ` Dan Carpenter
2017-05-19 19:47   ` Dan Carpenter [this message]
2017-05-19 20:08     ` Eric Anholt
2017-05-19 20:16       ` Dan Carpenter
2017-05-23  8:19         ` Jani Nikula
2017-05-23  8:31           ` Dan Carpenter
2017-05-23 13:01             ` Daniel Vetter

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=20170519194759.gfiefx6xwrnrjn3x@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=airlied@linux.ie \
    --cc=colin.king@canonical.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=eric@anholt.net \
    --cc=jani.nikula@linux.intel.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tom.cooksey@arm.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;
as well as URLs for NNTP newsgroup(s).