2015-04-07 09:12:55

by Ingo Molnar

[permalink] [raw]
Subject: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)


* Joe Perches <[email protected]> wrote:

> On Tue, 2015-03-31 at 11:03 +0200, Ingo Molnar wrote:
> > * Peter Zijlstra <[email protected]> wrote:
> > > On Mon, Mar 30, 2015 at 04:46:17PM -0700, Joe Perches wrote:
> > > > Use the normal return values for bool functions
> > > >
> > > > Update the other sets of ret in try_wait_for_completion.
> > >
> > > I'm missing a why; why are you doing this?
> >
> > Let me guess: Joe Perches is suffering from 'trivialititis': a
> > sickness that prevents a non-newbie kernel developer from raising
> > beyond churning out a flood of trivial patches and creating
> > unnecessary churn for other developers with these borderline
> > useless patches?
> >
> > Linux is a meritocracy, not a bureaucracy.
>
> Good morning Ingo.
>
> As you are a signer of that "code of conflict" patch,
> I'll be mildly amused, but not surprised, if you are
> among the first participants.

So as a reply to my joke directed against your (costly: see below)
flood of trivial and somewhat bureaucratic patches that PeterZ
complained about, which reply of mine aimed at getting you to change
from your many years old pattern of producing trivial patches towards
producing more substantial patches, causes you to issue a threat of
bureaucratic action against me?

Wow.

I'd also like to stress that I don't think you have answered PeterZ's
legitimate technical question adequately: what are the technological
justifications for doing this 25 patches series - returning 0/1 or
true/false is clearly a matter of taste unless mixed within the same
function. In fact what are your technological justifications for doing
so many trivial patches in general?

Please don't bother producing and sending me such trivial patches
unless they:

- fix a real bug (in which case they are not trivial patches anymore)

- or are part of a larger (non-trivial!) series that does some real,
substantial work on this code that tries to:

- fix existing code

- speed up existing code

- or expand upon existing code with new code

- turn totally unreadable code into something readable
(for example in drivers/staging/)

The reason I'm not applying your patch is that trivial patches, even
if they seem borderline useful, with no substance following them up,
often have more costs than benefits:

- they lead to pointless churn:

- they take up Git space (and bandwidth) for no good reason

- they slow down bisection of real changes

- they take up (valuable!) reviewer bandwidth

- they take up maintainer bandwidth

there's literally a million borderline pointless cleanup patches that
could be done on the kernel, and we really don't want to add a million
commits to the kernel tree...

I don't think your 25 patches long trivial series is defensible from a
kernel contributor who has thousands of commits in the mainline kernel
already: you are clearly not a kernel newbie anymore who needs to
learn the ropes through simple patches and whose initially trivial
patches maintainers will nurture in the hope of gaining a future
contributor who will be a net benefit in the future...

I also think you are beginning to abuse the openness of kernel
maintainers to apply trivial patches, and I don't think it's useful to
point out such abuse before it gets worse.

My (repeated) advice to you is that you should try to raise beyond
newbie patches and write something more substantial that helps Linux:

- take a look at the many bugs on bugzilla.kernel.org and try to
analyze, reproduce or fix them

- go read kernel code, understand it and try to find real bugs.

- go test the latest kernels and find bugs in it. The fresher the
code, the more likely it is that it has bugs.

- go read kernel code and try to expand upon it

Fortunately it's not hard to contribute to the kernel once you are
beyond the 'newbie' status: there's literally an infinite amount of
work to be done on the kernel, and I welcome productive contributions
- but churning out trivial patches with no substantial patches
following them up is not productive and in fact (as pointed out above)
they are harmful once you are not a totally fresh newbie kernel
developer anymore...

Pointing out this truth and protecting against such abusive flood of
trivial patches is not against the code of conduct I signed.

Thanks,

Ingo


2015-04-07 09:27:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)


* Ingo Molnar <[email protected]> wrote:

>
> * Joe Perches <[email protected]> wrote:
>
> > On Tue, 2015-03-31 at 11:03 +0200, Ingo Molnar wrote:
> > > * Peter Zijlstra <[email protected]> wrote:
> > > > On Mon, Mar 30, 2015 at 04:46:17PM -0700, Joe Perches wrote:
> > > > > Use the normal return values for bool functions
> > > > >
> > > > > Update the other sets of ret in try_wait_for_completion.
> > > >
> > > > I'm missing a why; why are you doing this?
> > >
> > > Let me guess: Joe Perches is suffering from 'trivialititis': a
> > > sickness that prevents a non-newbie kernel developer from raising
> > > beyond churning out a flood of trivial patches and creating
> > > unnecessary churn for other developers with these borderline
> > > useless patches?
> > >
> > > Linux is a meritocracy, not a bureaucracy.
> >
> > Good morning Ingo.
> >
> > As you are a signer of that "code of conflict" patch,
> > I'll be mildly amused, but not surprised, if you are
> > among the first participants.
>
> So as a reply to my joke directed against your (costly: see below)
> flood of trivial and somewhat bureaucratic patches that PeterZ
> complained about, which reply of mine aimed at getting you to change
> from your many years old pattern of producing trivial patches towards
> producing more substantial patches, causes you to issue a threat of
> bureaucratic action against me?
>
> Wow.
>
> I'd also like to stress that I don't think you have answered PeterZ's
> legitimate technical question adequately: what are the technological
> justifications for doing this 25 patches series - returning 0/1 or
> true/false is clearly a matter of taste unless mixed within the same
> function. In fact what are your technological justifications for doing
> so many trivial patches in general?
>
> Please don't bother producing and sending me such trivial patches
> unless they:
>
> - fix a real bug (in which case they are not trivial patches anymore)
>
> - or are part of a larger (non-trivial!) series that does some real,
> substantial work on this code that tries to:
>
> - fix existing code
>
> - speed up existing code
>
> - or expand upon existing code with new code
>
> - turn totally unreadable code into something readable
> (for example in drivers/staging/)
>
> The reason I'm not applying your patch is that trivial patches, even
> if they seem borderline useful, with no substance following them up,
> often have more costs than benefits:
>
> - they lead to pointless churn:
>
> - they take up Git space (and bandwidth) for no good reason
>
> - they slow down bisection of real changes
>
> - they take up (valuable!) reviewer bandwidth
>
> - they take up maintainer bandwidth

Not to mention that one of your trivial patches in this series
actually introduced a bug:

lkml.kernel.org/r/[email protected]

Which additional risk adds to the cost side of the equation as well.

Thanks,

Ingo

2015-04-07 09:36:29

by Ingo Molnar

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conflict (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)


s/Conduct/Conflict

* Ingo Molnar <[email protected]> wrote:

>
> * Joe Perches <[email protected]> wrote:
>
> > On Tue, 2015-03-31 at 11:03 +0200, Ingo Molnar wrote:
> > > * Peter Zijlstra <[email protected]> wrote:
> > > > On Mon, Mar 30, 2015 at 04:46:17PM -0700, Joe Perches wrote:
> > > > > Use the normal return values for bool functions
> > > > >
> > > > > Update the other sets of ret in try_wait_for_completion.
> > > >
> > > > I'm missing a why; why are you doing this?
> > >
> > > Let me guess: Joe Perches is suffering from 'trivialititis': a
> > > sickness that prevents a non-newbie kernel developer from raising
> > > beyond churning out a flood of trivial patches and creating
> > > unnecessary churn for other developers with these borderline
> > > useless patches?
> > >
> > > Linux is a meritocracy, not a bureaucracy.
> >
> > Good morning Ingo.
> >
> > As you are a signer of that "code of conflict" patch,
> > I'll be mildly amused, but not surprised, if you are
> > among the first participants.
>
> So as a reply to my joke directed against your (costly: see below)
> flood of trivial and somewhat bureaucratic patches that PeterZ
> complained about, which reply of mine aimed at getting you to change
> from your many years old pattern of producing trivial patches towards
> producing more substantial patches, causes you to issue a threat of
> bureaucratic action against me?
>
> Wow.
>
> I'd also like to stress that I don't think you have answered PeterZ's
> legitimate technical question adequately: what are the technological
> justifications for doing this 25 patches series - returning 0/1 or
> true/false is clearly a matter of taste unless mixed within the same
> function. In fact what are your technological justifications for doing
> so many trivial patches in general?
>
> Please don't bother producing and sending me such trivial patches
> unless they:
>
> - fix a real bug (in which case they are not trivial patches anymore)
>
> - or are part of a larger (non-trivial!) series that does some real,
> substantial work on this code that tries to:
>
> - fix existing code
>
> - speed up existing code
>
> - or expand upon existing code with new code
>
> - turn totally unreadable code into something readable
> (for example in drivers/staging/)
>
> The reason I'm not applying your patch is that trivial patches, even
> if they seem borderline useful, with no substance following them up,
> often have more costs than benefits:
>
> - they lead to pointless churn:
>
> - they take up Git space (and bandwidth) for no good reason
>
> - they slow down bisection of real changes
>
> - they take up (valuable!) reviewer bandwidth
>
> - they take up maintainer bandwidth
>
> there's literally a million borderline pointless cleanup patches that
> could be done on the kernel, and we really don't want to add a million
> commits to the kernel tree...
>
> I don't think your 25 patches long trivial series is defensible from a
> kernel contributor who has thousands of commits in the mainline kernel
> already: you are clearly not a kernel newbie anymore who needs to
> learn the ropes through simple patches and whose initially trivial
> patches maintainers will nurture in the hope of gaining a future
> contributor who will be a net benefit in the future...
>
> I also think you are beginning to abuse the openness of kernel
> maintainers to apply trivial patches, and I don't think it's useful to
> point out such abuse before it gets worse.
>
> My (repeated) advice to you is that you should try to raise beyond
> newbie patches and write something more substantial that helps Linux:
>
> - take a look at the many bugs on bugzilla.kernel.org and try to
> analyze, reproduce or fix them
>
> - go read kernel code, understand it and try to find real bugs.
>
> - go test the latest kernels and find bugs in it. The fresher the
> code, the more likely it is that it has bugs.
>
> - go read kernel code and try to expand upon it
>
> Fortunately it's not hard to contribute to the kernel once you are
> beyond the 'newbie' status: there's literally an infinite amount of
> work to be done on the kernel, and I welcome productive contributions
> - but churning out trivial patches with no substantial patches
> following them up is not productive and in fact (as pointed out above)
> they are harmful once you are not a totally fresh newbie kernel
> developer anymore...
>
> Pointing out this truth and protecting against such abusive flood of
> trivial patches is not against the 'Code of Conflict' I signed.
>
> Thanks,
>
> Ingo

2015-04-07 09:39:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conflict (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)


* Ingo Molnar <[email protected]> wrote:

> > I also think you are beginning to abuse the openness of kernel
> > maintainers to apply trivial patches, and I don't think it's
> > useful to point out such abuse before it gets worse.

and this should read:

> > I also think you are beginning to abuse the openness of kernel
> > maintainers to apply trivial patches, and I don't think it's
> > harmful to point out such abuse before it gets worse.

Thanks,

Ingo

2015-04-07 11:00:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 07, 2015 at 11:12:46AM +0200, Ingo Molnar wrote:
> Pointing out this truth and protecting against such abusive flood of
> trivial patches is not against the code of conduct I signed.

I totally agree, it's not "against" the code of conflict that I helped
write.

Joe, you know better than to send trivial stuff to maintainers who don't
want it. Send it through the trivial maintainer for subsystems that
have expressed annoyance at this, it's not the first time this has
happened.

Some maintainers, like me, are fine with your types of patches, I'd
stick to those subsystems if you like doing this type of work.

Also, don't send stuff that is broken, that's not the sign of a trivial
patch :(

thanks,

greg k-h

2015-04-07 11:19:03

by Ingo Molnar

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)


* Greg Kroah-Hartman <[email protected]> wrote:

> On Tue, Apr 07, 2015 at 11:12:46AM +0200, Ingo Molnar wrote:
> > Pointing out this truth and protecting against such abusive flood of
> > trivial patches is not against the code of conduct I signed.
>
> I totally agree, it's not "against" the code of conflict that I
> helped write.
>
> Joe, you know better than to send trivial stuff to maintainers who
> don't want it. Send it through the trivial maintainer for
> subsystems that have expressed annoyance at this, it's not the first
> time this has happened.

I argue that they should not be sent _at all_ in such cases, not even
via the trivial tree: firstly because typically I'll pick up the bits
from the trivial tree as well, and secondly because most of the
arguments I listed against bulk trivial commits (weaker bisectability,
taking up reviewer bandwidth, taking up Git space, etc.) still stand.

Frankly IMHO such a */25 series could be a net negative contribution
when coming from a kernel contributor who has written 2000+ trivial
patches already...

> Some maintainers, like me, are fine with your types of patches, I'd
> stick to those subsystems if you like doing this type of work.

So sending trivial patches for things like totally unreadable code in
say drivers/staging/ is probably OK, as they materially transform the
code and make it more maintainable.

For the rest it can be more harmful than beneficial, for the reasons I
outlined.

Thanks,

Ingo

2015-04-07 11:27:42

by Peter Zijlstra

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 07, 2015 at 01:18:56PM +0200, Ingo Molnar wrote:
>
> * Greg Kroah-Hartman <[email protected]> wrote:
>
> > On Tue, Apr 07, 2015 at 11:12:46AM +0200, Ingo Molnar wrote:
> > > Pointing out this truth and protecting against such abusive flood of
> > > trivial patches is not against the code of conduct I signed.
> >
> > I totally agree, it's not "against" the code of conflict that I
> > helped write.
> >
> > Joe, you know better than to send trivial stuff to maintainers who
> > don't want it. Send it through the trivial maintainer for
> > subsystems that have expressed annoyance at this, it's not the first
> > time this has happened.
>
> I argue that they should not be sent _at all_ in such cases, not even
> via the trivial tree: firstly because typically I'll pick up the bits
> from the trivial tree as well, and secondly because most of the
> arguments I listed against bulk trivial commits (weaker bisectability,
> taking up reviewer bandwidth, taking up Git space, etc.) still stand.

I agree, I do not want actual code changes to by-pass me for the
subsystems I'm responsible for.

Typoes in comments I can live with, but I want to see each and every
patch that changes actual code.

2015-04-07 11:28:30

by Richard Weinberger

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 7, 2015 at 1:00 PM, Greg Kroah-Hartman
<[email protected]> wrote:
> On Tue, Apr 07, 2015 at 11:12:46AM +0200, Ingo Molnar wrote:
>> Pointing out this truth and protecting against such abusive flood of
>> trivial patches is not against the code of conduct I signed.
>
> I totally agree, it's not "against" the code of conflict that I helped
> write.
>
> Joe, you know better than to send trivial stuff to maintainers who don't
> want it. Send it through the trivial maintainer for subsystems that
> have expressed annoyance at this, it's not the first time this has
> happened.
>
> Some maintainers, like me, are fine with your types of patches, I'd
> stick to those subsystems if you like doing this type of work.

Can't we send all these kind of patches through the trivial tree?
Don't get me wrong, if you are fine with these patches that's you decision.
But other maintainers might think they have to take these patches and
get overloaded. I'm thinking of drivers maintainers that can only work
one or two hours per week on Linux.
Not everyone works full time on it like you.

I propose to send all this stuff though the trivial tree such that maintainers
of other subsystems have less workload and newbies (which are supposed
to send such patches) know which tree they have to work against.
Let's have to well defined and ordered. :-)

--
Thanks,
//richard

2015-04-07 11:32:29

by Peter Zijlstra

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 07, 2015 at 01:28:27PM +0200, Richard Weinberger wrote:
> Can't we send all these kind of patches through the trivial tree?
> Don't get me wrong, if you are fine with these patches that's you decision.
> But other maintainers might think they have to take these patches and
> get overloaded. I'm thinking of drivers maintainers that can only work
> one or two hours per week on Linux.
> Not everyone works full time on it like you.
>
> I propose to send all this stuff though the trivial tree such that maintainers
> of other subsystems have less workload and newbies (which are supposed
> to send such patches) know which tree they have to work against.
> Let's have to well defined and ordered. :-)

As per the other branch of this tree; an emphatic NO to that. The
trivial tree is not a backdoor to bypass maintainers. Actual code
changes do not get to go through any tree but the maintainer tree unless
explicitly ACKed.

2015-04-07 11:50:39

by Richard Weinberger

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct

Am 07.04.2015 um 13:32 schrieb Peter Zijlstra:
> On Tue, Apr 07, 2015 at 01:28:27PM +0200, Richard Weinberger wrote:
>> Can't we send all these kind of patches through the trivial tree?
>> Don't get me wrong, if you are fine with these patches that's you decision.
>> But other maintainers might think they have to take these patches and
>> get overloaded. I'm thinking of drivers maintainers that can only work
>> one or two hours per week on Linux.
>> Not everyone works full time on it like you.
>>
>> I propose to send all this stuff though the trivial tree such that maintainers
>> of other subsystems have less workload and newbies (which are supposed
>> to send such patches) know which tree they have to work against.
>> Let's have to well defined and ordered. :-)
>
> As per the other branch of this tree; an emphatic NO to that. The
> trivial tree is not a backdoor to bypass maintainers. Actual code
> changes do not get to go through any tree but the maintainer tree unless
> explicitly ACKed.

I agree that the series in question is useless.
But if a patch is trivial it can go through the trivial tree.
By trivial I really mean *trivial* in terms of typos
and 80 character limit crap.
It has to be something which does not hurt and the maintainer
can safely ignore.

Thanks,
//richard

2015-04-07 12:07:03

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tuesday, April 07, 2015 01:32:12 PM Peter Zijlstra wrote:
> On Tue, Apr 07, 2015 at 01:28:27PM +0200, Richard Weinberger wrote:
> > Can't we send all these kind of patches through the trivial tree?
> > Don't get me wrong, if you are fine with these patches that's you decision.
> > But other maintainers might think they have to take these patches and
> > get overloaded. I'm thinking of drivers maintainers that can only work
> > one or two hours per week on Linux.
> > Not everyone works full time on it like you.
> >
> > I propose to send all this stuff though the trivial tree such that maintainers
> > of other subsystems have less workload and newbies (which are supposed
> > to send such patches) know which tree they have to work against.
> > Let's have to well defined and ordered. :-)
>
> As per the other branch of this tree; an emphatic NO to that. The
> trivial tree is not a backdoor to bypass maintainers. Actual code
> changes do not get to go through any tree but the maintainer tree unless
> explicitly ACKed.

Well, practically speaking, that would make changes like the recent
clockevents_notify() removal very difficult to carry out. Also there is
some natural cross-talk between certain subsystems.

Different matter is the real value of tree-wide cleanup changes. If code is
old enough it often is better to leave it alone, even though it may be doing
things that we don't usually do nowadays.

Or things that new patches are not supposed to do, for that matter, so
I generally don't like the "checkpatch.pl error fix" changes in the old code.

Rafael

2015-04-07 12:35:09

by Joe Perches

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, 2015-04-07 at 11:12 +0200, Ingo Molnar wrote:
> I don't think you have answered PeterZ's
> legitimate technical question adequately:

As I wrote before, ~13000:180 is a big ratio.
http://www.kernelhub.org/?p=2&msg=718145

> what are the technological
> justifications for doing this 25 patches series - returning 0/1 or
> true/false is clearly a matter of taste unless mixed within the same
> function.

I presume you mean file rather than function.

21 of the 55 files modified already used "return true"
or "return false" for other bool function returns.

Many of the 1/0 uses were in functions where the
function return type was changed from int to bool but
the return statements were unchanged.

Expectation and consistency matter.
To what degree is, as always, debatable.

Code that causes a reader to "stall" when reading is
generally not good.

Patches that span subsystems, given the distribution
of interest and responsibilities in the kernel,
generally aren't possible to apply by any single
maintainer.

It's generally necessary to split even these admittedly
trivial patches by area of maintainership.

Bypassing actual maintainers by sending only to the
nominal trivial maintainer isn't good.

> In fact what are your technological justifications for doing
> so many trivial patches in general?

Consistency improves reading speed and can help reduce
future defects.

> Please don't bother producing and sending me such trivial patches

My creation of these types of patches either singly
or in a series is fairly automated.

I will sed your name and email address out of the
scripts that generate the "To:" and "CC:" lists for
patches in the future.

If any other person with a MAINTAINERS entry wants not
to receive these types of patches from me, please let
me know privately.

> Pointing out this truth and protecting against such abusive flood of
> trivial patches is not against the code of conduct I signed.

Jokes are hard.
"Be excellent to each other" can be too, but shouldn't be.
Manners and civility are important.
Yours, like mine, can always be improved.
We all should strive for that improvement.

cheers, Joe

2015-04-07 13:21:33

by Steven Rostedt

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct

On Tue, Apr 07, 2015 at 01:50:31PM +0200, Richard Weinberger wrote:
> >
> > As per the other branch of this tree; an emphatic NO to that. The
> > trivial tree is not a backdoor to bypass maintainers. Actual code
> > changes do not get to go through any tree but the maintainer tree unless
> > explicitly ACKed.
>
> I agree that the series in question is useless.
> But if a patch is trivial it can go through the trivial tree.

Only if they received an Acked-by from the maintainer of the code that
it touches. That way, Peter does see the code that is changing. He doesn't
need to take it through his tree, but the trivial maintainer must get his
Acked-by, which shows that he did actually take a look at the patch and is
fine with it going through another route.


> By trivial I really mean *trivial* in terms of typos
> and 80 character limit crap.

Egad no. The 80 character limit is a guideline not set in stone. There's so
many times I see people break up lines to avoid that limit and make the
code uglier and more difficult to read. Again, that's a trivial change that
would do more harm than good.

> It has to be something which does not hurt and the maintainer
> can safely ignore.

I think the only change that could probably go in without an ack from the
maintainer is a change that Peter already mentioned. Typos in comments that
do not touch the actual code.

-- Steve

2015-04-07 13:22:08

by Peter Hurley

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct

On 04/07/2015 07:18 AM, Ingo Molnar wrote:
> * Greg Kroah-Hartman <[email protected]> wrote:
>
>> On Tue, Apr 07, 2015 at 11:12:46AM +0200, Ingo Molnar wrote:
>>> Pointing out this truth and protecting against such abusive flood of
>>> trivial patches is not against the code of conduct I signed.
>>
>> I totally agree, it's not "against" the code of conflict that I
>> helped write.
>>
>> Joe, you know better than to send trivial stuff to maintainers who
>> don't want it. Send it through the trivial maintainer for
>> subsystems that have expressed annoyance at this, it's not the first
>> time this has happened.
>
> I argue that they should not be sent _at all_ in such cases, not even
> via the trivial tree: firstly because typically I'll pick up the bits
> from the trivial tree as well, and secondly because most of the
> arguments I listed against bulk trivial commits (weaker bisectability,
> taking up reviewer bandwidth, taking up Git space, etc.) still stand.

And requires backports for -stable.

Regards,
Peter Hurley

2015-04-07 13:29:14

by Steven Rostedt

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 07, 2015 at 02:31:23PM +0200, Rafael J. Wysocki wrote:
> >
> > As per the other branch of this tree; an emphatic NO to that. The
> > trivial tree is not a backdoor to bypass maintainers. Actual code
> > changes do not get to go through any tree but the maintainer tree unless
> > explicitly ACKed.
>
> Well, practically speaking, that would make changes like the recent
> clockevents_notify() removal very difficult to carry out. Also there is
> some natural cross-talk between certain subsystems.

I would not call the clockevents_notify() series "trivial". More advanced
clean ups that are system wide, would be different, because you are changing
the way the code works. The maintainers must be Cc'd, but sometimes I find
those changes are very hard to get acks from everyone. But again, the change
is a non trivial clean up and has other reasons for going in than just to
make the code look nice.

>
> Different matter is the real value of tree-wide cleanup changes. If code is
> old enough it often is better to leave it alone, even though it may be doing
> things that we don't usually do nowadays.

Or maybe it's a good time to rewrite that code such that everyone can understand
it today ;-)

>
> Or things that new patches are not supposed to do, for that matter, so
> I generally don't like the "checkpatch.pl error fix" changes in the old code.
>

I totally agree with that. But for non trivial clean ups, old code should be
updated too.

-- Steve

2015-04-07 13:28:24

by Richard Weinberger

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct

Am 07.04.2015 um 15:21 schrieb Steven Rostedt:
> On Tue, Apr 07, 2015 at 01:50:31PM +0200, Richard Weinberger wrote:
>>>
>>> As per the other branch of this tree; an emphatic NO to that. The
>>> trivial tree is not a backdoor to bypass maintainers. Actual code
>>> changes do not get to go through any tree but the maintainer tree unless
>>> explicitly ACKed.
>>
>> I agree that the series in question is useless.
>> But if a patch is trivial it can go through the trivial tree.
>
> Only if they received an Acked-by from the maintainer of the code that
> it touches. That way, Peter does see the code that is changing. He doesn't
> need to take it through his tree, but the trivial maintainer must get his
> Acked-by, which shows that he did actually take a look at the patch and is
> fine with it going through another route.
>
>
>> By trivial I really mean *trivial* in terms of typos
>> and 80 character limit crap.
>
> Egad no. The 80 character limit is a guideline not set in stone. There's so
> many times I see people break up lines to avoid that limit and make the
> code uglier and more difficult to read. Again, that's a trivial change that
> would do more harm than good.

That's why i named it crap. :D

>> It has to be something which does not hurt and the maintainer
>> can safely ignore.
>
> I think the only change that could probably go in without an ack from the
> maintainer is a change that Peter already mentioned. Typos in comments that
> do not touch the actual code.

Agreed.

Thanks,
//richard

2015-04-08 03:22:54

by Theodore Ts'o

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tue, Apr 07, 2015 at 01:32:12PM +0200, Peter Zijlstra wrote:
> > I propose to send all this stuff though the trivial tree such that maintainers
> > of other subsystems have less workload and newbies (which are supposed
> > to send such patches) know which tree they have to work against.
> > Let's have to well defined and ordered. :-)
>
> As per the other branch of this tree; an emphatic NO to that. The
> trivial tree is not a backdoor to bypass maintainers. Actual code
> changes do not get to go through any tree but the maintainer tree unless
> explicitly ACKed.

Agreed, I don't want trivial patches to ext4 either (a) polluting my
inbox, or (b) sneaking in behind my back in the trivial tree.

Joe, please just stop the madness.

Thanks,

- Ted

2015-04-08 23:13:08

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Tuesday, April 07, 2015 09:28:03 AM Steven Rostedt wrote:
> On Tue, Apr 07, 2015 at 02:31:23PM +0200, Rafael J. Wysocki wrote:
> > >
> > > As per the other branch of this tree; an emphatic NO to that. The
> > > trivial tree is not a backdoor to bypass maintainers. Actual code
> > > changes do not get to go through any tree but the maintainer tree unless
> > > explicitly ACKed.
> >
> > Well, practically speaking, that would make changes like the recent
> > clockevents_notify() removal very difficult to carry out. Also there is
> > some natural cross-talk between certain subsystems.
>
> I would not call the clockevents_notify() series "trivial". More advanced
> clean ups that are system wide, would be different, because you are changing
> the way the code works. The maintainers must be Cc'd, but sometimes I find
> those changes are very hard to get acks from everyone. But again, the change
> is a non trivial clean up and has other reasons for going in than just to
> make the code look nice.
>
> >
> > Different matter is the real value of tree-wide cleanup changes. If code is
> > old enough it often is better to leave it alone, even though it may be doing
> > things that we don't usually do nowadays.
>
> Or maybe it's a good time to rewrite that code such that everyone can understand
> it today ;-)
>
> >
> > Or things that new patches are not supposed to do, for that matter, so
> > I generally don't like the "checkpatch.pl error fix" changes in the old code.
> >
>
> I totally agree with that. But for non trivial clean ups, old code should be
> updated too.

Well, there still is some risk of introducing real bugs this way (I *have* seen
"trivial cleanups" that broke things) and is it really worth it?

Besides, old code is somewhat like an ancient building. Yes, it needs to be
kept in a good shape, but you won't replace bricks in it just because they are
old, will you?

Rafael

2015-04-09 00:04:29

by Joe Perches

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

On Thu, 2015-04-09 at 01:37 +0200, Rafael J. Wysocki wrote:
> old code is somewhat like an ancient building. Yes, it needs to be
> kept in a good shape, but you won't replace bricks in it just because they are
> old, will you?

No, but you do have to replace/repoint the mortar
as it ages.

Here in SoCal, we also have to do structural work
on UnReinforced Masonry (URM) buildings to avoid
loss of life when the ground moves underneath us.

A small story:

There's a guy a few blocks from me that wants to
open a restaurant in a ~70 year old building. He's
been fighting the city permitting process for over
a year now because he rented a well built, but old,
URM building with double courses of brick walls
that had already had some structural improvements.

http://la.eater.com/2014/3/18/6260975/brixton-restaurant-going-into-the-joker-in-santa-monica

He added some windows where previously windows had
been located but also had been covered and filled-in
with over time. The place had been a dark, smoky
dive bar.

City went into a mode where any change to the
building was unacceptable unless _every_ element
of the building was brought up to new construction
standards.

It's a silly and expensive process for him and an
overall loss for the city because of delays in
creation of a tax-paying business.

Santa Monica, my home town...

Anyway, code isn't a lot like a building.

It's easy to prove that some source code change
doesn't cause a change in object output.

It's not so easy in a physical structure.

2015-04-13 12:19:05

by Alan Cox

[permalink] [raw]
Subject: Re: about the flood of trivial patches and the Code of Conduct (was: Re: [PATCH 19/25] sched: Use bool function return values of true/false not 1/0)

> Besides, old code is somewhat like an ancient building. Yes, it needs to be
> kept in a good shape, but you won't replace bricks in it just because they are
> old, will you?

When they matter to the integrity and they are likely to be full of
internal cracks and holes you do.

What concerns me more is that we now have lots of old driver code in the
kernel that nobody uses, that nobody cares about in reality but which has
been turd polished to the point that at casual glance it looks maintained
and useful but probably hasn't been tested in years.

Alan