From: Dave Gordon <david.s.gordon@intel.com>
To: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Make sure our labels start at column 0
Date: Mon, 15 Jun 2015 19:18:24 +0100 [thread overview]
Message-ID: <557F16F0.8000506@intel.com> (raw)
In-Reply-To: <20150615123434.GO8341@phenom.ffwll.local>
On 15/06/15 13:34, Daniel Vetter wrote:
> On Fri, Jun 05, 2015 at 12:27:21PM +0300, Ville Syrjälä wrote:
>> On Fri, Jun 05, 2015 at 12:24:45PM +0300, Jani Nikula wrote:
>>> On Thu, 04 Jun 2015, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>>> On Thu, Jun 04, 2015 at 04:56:18PM +0100, Damien Lespiau wrote:
>>>>> I noticed one of those and it turned out we have a few lingering around.
>>>>
>>>> Yuck. I'd prefer we got the other way. Consider the following diffs for example:
>>>
>>> What's the, uh, diff between those to consider?
>>
>> Look at the @@ line. One tells you in which function the line is added,
>> the other one doesn't. It always pisses me off when reviewing patches
>> cause then I have to figure out the function based on the label,
>> surroundng context, and/or line numbers.
>
> Yeah that's an annoying sucker but I guess just part of the fail. Imo
> consistency wins this bikeshed ;-)
>
>> I'm also thinking this may have caused some of the numerous misapplied
>> patches we've had since our labels all tend to be similar.
>
> Diff doesn't look at the heading after the @@ but only at concept. And
> when applying with some mismatches that can end up in really surprising
> places. Chaning how we place labels won't help.
> -Daniel
You could vary the label by giving each one some compressed prefix based
on the name of the function it's in, a sort of poor man's namespacing ...
i915_do_some_stuff()
{
...
goto dss_exit;
...
dss_exit:
return ret;
}
i915_exciting_new_function()
{
...
goto enf_exit;
...
enf_exit:
return ret;
}
.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2015-06-15 18:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-04 15:56 [PATCH] drm/i915: Make sure our labels start at column 0 Damien Lespiau
2015-06-04 16:34 ` Ville Syrjälä
2015-06-05 9:24 ` Jani Nikula
2015-06-05 9:27 ` Ville Syrjälä
2015-06-05 10:04 ` Damien Lespiau
2015-06-05 13:47 ` Dave Gordon
2015-06-15 12:34 ` Daniel Vetter
2015-06-15 18:18 ` Dave Gordon [this message]
2015-06-05 14:51 ` shuang.he
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=557F16F0.8000506@intel.com \
--to=david.s.gordon@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/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.