Cleanups in "next" tree

Cleanups in "next" tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

Hi!

I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
branch for-next ), if someone wants to review them.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Cleanups in "next" tree

[Jacek Anaszewski]

Hi Pavel,

On 3/22/20 12:59 PM, Pavel Machek wrote:
> Hi!
> 
> I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
> branch for-next ), if someone wants to review them.

You abused your maintainer power by bypassing the usual patch
submission procedure. Please remove the patches from linux-next
and submit them officially for discussion. I would have some objections
to them.

-- 
Best regards,
Jacek Anaszewski

Re: Cleanups in "next" tree

[Dan Murphy]

Pavel

On 3/22/20 8:35 AM, Jacek Anaszewski wrote:
> Hi Pavel,
>
> On 3/22/20 12:59 PM, Pavel Machek wrote:
>> Hi!
>>
>> I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
>> branch for-next ), if someone wants to review them.
> You abused your maintainer power by bypassing the usual patch
> submission procedure. Please remove the patches from linux-next
> and submit them officially for discussion. I would have some objections
> to them.
>
I had a comment on one of the patches and wanted to add my RB.

So can you please post them?

Dan

Re: Cleanups in "next" tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 861 bytes --]

On Sun 2020-03-22 14:35:56, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 3/22/20 12:59 PM, Pavel Machek wrote:
> > Hi!
> > 
> > I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
> > branch for-next ), if someone wants to review them.
> 
> You abused your maintainer power by bypassing the usual patch
> submission procedure. Please remove the patches from linux-next
> and submit them officially for discussion. I would have some objections
> to them.

I'm sorry I failed to meet your high expectations... But I don't
believe I done anything completely outside of usual kernel procedures.

Could you list the patches and objections you have?

Thanks and best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: Cleanups in "next" tree

[Dan Murphy]

Pavel

On 4/2/20 5:57 PM, Pavel Machek wrote:
> On Sun 2020-03-22 14:35:56, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 3/22/20 12:59 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>> I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
>>> branch for-next ), if someone wants to review them.
>> You abused your maintainer power by bypassing the usual patch
>> submission procedure. Please remove the patches from linux-next
>> and submit them officially for discussion. I would have some objections
>> to them.
> I'm sorry I failed to meet your high expectations... But I don't
> believe I done anything completely outside of usual kernel procedures.

So I can push a public tree out and request reviewers to review that 
tree and expect it to get merged once the review is complete without 
ever posting the patches to linux-leds?

This would be the precedent you are setting here as maintainer.

And Jacek does not have high expectations he is just requesting that we 
follow the process as defined in the Linux kernel document

Dan

Re: Cleanups in "next" tree

[Jacek Anaszewski]

On 4/3/20 12:57 AM, Pavel Machek wrote:
> On Sun 2020-03-22 14:35:56, Jacek Anaszewski wrote:
>> Hi Pavel,
>>
>> On 3/22/20 12:59 PM, Pavel Machek wrote:
>>> Hi!
>>>
>>> I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
>>> branch for-next ), if someone wants to review them.
>>
>> You abused your maintainer power by bypassing the usual patch
>> submission procedure. Please remove the patches from linux-next
>> and submit them officially for discussion. I would have some objections
>> to them.
> 
> I'm sorry I failed to meet your high expectations... But I don't
> believe I done anything completely outside of usual kernel procedures.

I believe code review is quite usual kernel procedure.

> Could you list the patches and objections you have?

I already expressed my concerns regarding Turris Omnia patch.

My comments regarding remaining patches:

- "Make label "white:power" to be consistent with"

I disagree here. "system" was OK.

- "Warn about old defines that probably should not be used."

Obsolete is only LED_FULL, so the comment is in wrong line

- "Group LED functions according to functionality, and add some"

You're adding here some random comments referencing obsolete
naming. I think that it is enough to say what is current standard.

Also, I had a patch [0] describing standard LED functions in my LED
naming patch set, but it was not merged. It could be worth getting
back to it at this occasion.


[0]
https://lore.kernel.org/linux-leds/20190609190803.14815-27-jacek.anaszewski@gmail.com/

-- 
Best regards,
Jacek Anaszewski

Re: Cleanups in "next" tree

[Dan Murphy]

Pavel

On 4/3/20 1:45 PM, Jacek Anaszewski wrote:
> On 4/3/20 12:57 AM, Pavel Machek wrote:
>> On Sun 2020-03-22 14:35:56, Jacek Anaszewski wrote:
>>> Hi Pavel,
>>>
>>> On 3/22/20 12:59 PM, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>> I've commited some cleanups into LED tree ( git/pavel/linux-leds.git
>>>> branch for-next ), if someone wants to review them.
>>> You abused your maintainer power by bypassing the usual patch
>>> submission procedure. Please remove the patches from linux-next
>>> and submit them officially for discussion. I would have some objections
>>> to them.
>> I'm sorry I failed to meet your high expectations... But I don't
>> believe I done anything completely outside of usual kernel procedures.
> I believe code review is quite usual kernel procedure.
>
>> Could you list the patches and objections you have?
> I already expressed my concerns regarding Turris Omnia patch.
>
> My comments regarding remaining patches:
>
> - "Make label "white:power" to be consistent with"
>
> I disagree here. "system" was OK.
>
> - "Warn about old defines that probably should not be used."
>
> Obsolete is only LED_FULL, so the comment is in wrong line

I would prefer to have the commit sha that obsoleted the LED_FULL to be 
referenced in the commit message so we have traceability.

Dan

Re: Cleanups in "next" tree

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

Hi!

> > I'm sorry I failed to meet your high expectations... But I don't
> > believe I done anything completely outside of usual kernel procedures.
> 
> I believe code review is quite usual kernel procedure.

I don't disagree with that.

> > Could you list the patches and objections you have?
> 
> I already expressed my concerns regarding Turris Omnia patch.

Ok.

> My comments regarding remaining patches:
> 
> - "Make label "white:power" to be consistent with"
> 
> I disagree here. "system" was OK.

It was too vague... I know the hardware and it is a LED above power
button used as a power indicator.

> - "Warn about old defines that probably should not be used."
> 
> Obsolete is only LED_FULL, so the comment is in wrong line

No, all of them are bad. Maybe LED_OFF could be used going forward,
but... it is simply easier to write 0. The type is not really an en
enumeration, it is brightness, with variable maximum value.

> - "Group LED functions according to functionality, and add some"
> 
> You're adding here some random comments referencing obsolete
> naming. I think that it is enough to say what is current standard.

Ok, I'll drop that part. But I really want to get that documented
_somewhere_, because obsolete naming is currently in use, and we won't
be able to change it :-(.

> Also, I had a patch [0] describing standard LED functions in my LED
> naming patch set, but it was not merged. It could be worth getting
> back to it at this occasion.

I'll take a look.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Cleanups in "next" tree

[Jacek Anaszewski]

On 4/7/20 9:28 AM, Pavel Machek wrote:
> Hi!
> 
>>> I'm sorry I failed to meet your high expectations... But I don't
>>> believe I done anything completely outside of usual kernel procedures.
>>
>> I believe code review is quite usual kernel procedure.
> 
> I don't disagree with that.
> 
>>> Could you list the patches and objections you have?
>>
>> I already expressed my concerns regarding Turris Omnia patch.
> 
> Ok.
> 
>> My comments regarding remaining patches:
>>
>> - "Make label "white:power" to be consistent with"
>>
>> I disagree here. "system" was OK.
> 
> It was too vague... I know the hardware and it is a LED above power
> button used as a power indicator.
> 
>> - "Warn about old defines that probably should not be used."
>>
>> Obsolete is only LED_FULL, so the comment is in wrong line
> 
> No, all of them are bad. Maybe LED_OFF could be used going forward,
> but... it is simply easier to write 0. The type is not really an en
> enumeration, it is brightness, with variable maximum value.

In this case brightness should be turned into an int and the change
should be applied throughout the whole kernel. Otherwise it is
questionable - why all enums are made obsolete if the type of struct
led_classdev's brightness property is still enum led_brightness?
Do we have some replacement? - one could ask.

> 
>> - "Group LED functions according to functionality, and add some"
>>
>> You're adding here some random comments referencing obsolete
>> naming. I think that it is enough to say what is current standard.
> 
> Ok, I'll drop that part. But I really want to get that documented
> _somewhere_, because obsolete naming is currently in use, and we won't
> be able to change it :-(.

You've pushed it out anyway...

>> Also, I had a patch [0] describing standard LED functions in my LED
>> naming patch set, but it was not merged. It could be worth getting
>> back to it at this occasion.
> 
> I'll take a look.
> 
> Best regards,
> 									Pavel
> 

-- 
Best regards,
Jacek Anaszewski