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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
