Re: [PATCH] backlight: remove obsolete comment for ->state

[Daniel Vetter]

Hi Lee,

On Wed, Jul 4, 2018 at 12:34 PM, Lee Jones <lee.jones@xxxxxxxxxx> wrote:
> On Wed, 04 Jul 2018, Daniel Vetter wrote:
>> On Wed, Jul 04, 2018 at 10:38:16AM +0100, Lee Jones wrote:
>> > On Wed, 04 Jul 2018, Lee Jones wrote:
>> >
>> > > > Jani spotted this when reviewing my earlier patch to remove the driver
>> > > > internal usage of this field in
>> > > >
>> > > > commit 3cf91adaa594e8933af1727942ac560e5c7bc70e
>> > > > Author: Daniel Vetter <daniel.vetter@xxxxxxxx>
>> > > > Date:   Wed Apr 25 19:42:52 2018 +0200
>> > > >
>> > > >     backlight: Nuke BL_CORE_DRIVER1
>> > >
>> > > FYI, sending patches like this is not a good idea.
>> > >
>> > > I'll clean it up for you this time, but in future please send patches
>> > > properly and place any additional comments you may have below the
>> > > '---' line.
>> >
>> > Ah, I see what you've tried to do.  This hurt my eyes! :)
>> >
>> > It's more conventional to reference commits like:
>> >
>> >   Commit 3cf91adaa594 ("backlight: Nuke BL_CORE_DRIVER1")
>> >
>> > Again, I'll make the amendment to avoid further confusion.
>>
>> So the first mail doesn't even bother to explain what's
>> objectionable
>
> In the first instance it looked as though you'd copied and pasted `git
> log`, which if you'd done so would have been obvious to you and would
> have required no further explanation.
>
> Also bear in mind that I took your standing within the kernel
> community into consideration, so speaking to you like a n00b or going
> into unnecessary detail could have been considered superfluous at best
> and condescending at worst.

Unfortunately minute details like this aren't consistent across the
kernel at all, and white space and layout issues are the number 1
reason I get shot at for random patches I'm sending out. So maybe
there are people who learned all these local expectations (Arnd
perhaps, or Kees?), it's definitely not me. Not after 10 years for
sure.

>> the 2nd mail still says "This hurts my eyes!".
>
> It certainly did, yes.
>
> Usually taken to meaning that it was hard to read in these scenarios.
>
>> Over whitespace in the commit message.
>
> Nothing to do with white space.  It was the format by which you chose
> to reference a previous commit.  Instead it looked like a patch
> formatting error.  I have received patches pasted from `git log`
> before, and this looked just the same.
>
> Once I'd realised what was going on, I followed up to explain and
> provided some feedback on what to do differently in future.
>
>> This kind of stuff is why graphics people really don't enjoy contributing
>> to the kernel at large. A friendly request to resend with the color choice
>> adjusted would go a lot further.
>
> I apologise if my brevity hurt your feelings.  I have 290 emails to
> get though post-paternity leave before I can start to think about
> getting some real/paid work done.

This ain't about my feelings, but working together efficiently and in
a constructive environment.

Also, failing to have adequate maintainer coverage over absence, or
generally being overloaded, is never a valid excuse for how you deal
with contributors. It takes some effort and a bit of time, but group
maintainership in one form or another can take care of this very well.
Brevity justified as efficient communication tends to torpedo that,
since at least in my experience it just drive prospective volunteers
away to more welcoming places.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch