2023-09-13 03:00:00

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote:
> On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote:
> > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote:
> > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote:
> > > > It's rather a gigantic list of heards that is very hard to follow.
> > > > Sorting helps to see what's already included and what's not.
> > > > It improves a maintainability in a long term.
> > > >
> > > > Signed-off-by: Andy Shevchenko <[email protected]>
> > >
> > > Hi Andy,
> > >
> > > At the risk of bike shedding, the sort function of Vim, when operating
> > > with the C locale, gives a slightly different order, as experssed by
> > > this incremental diff.
> > >
> > > I have no objections to your oder, but I'm slightly curious as
> > > to how it came about.
> >
> > !sort which is external command.
> >
> > $ locale -k LC_COLLATE
> > collate-nrules=4
> > collate-rulesets=""
> > collate-symb-hash-sizemb=1303
> > collate-codeset="UTF-8"
>
> I'm unsure this change is worthy. It will make any later fix touching
> the header list more difficult to backport, and I don't see a great
> direct advantage.

As Rasmus put it here
https://lore.kernel.org/lkml/[email protected]/
In short term you can argue that it's not beneficial, but in long term it's given
less conflicts.

> Please repost the first patch standalone.

Why to repost, what did I miss? It's available via lore, just run

b4 am -slt -P _ [email protected]

to get it :-)

--
With Best Regards,
Andy Shevchenko



2023-09-13 03:44:13

by Paolo Abeni

[permalink] [raw]
Subject: Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

On Tue, 2023-09-12 at 20:04 +0300, Andy Shevchenko wrote:
> On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote:
> > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote:
> > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote:
> > > > > It's rather a gigantic list of heards that is very hard to follow.
> > > > > Sorting helps to see what's already included and what's not.
> > > > > It improves a maintainability in a long term.
> > > > >
> > > > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > >
> > > > Hi Andy,
> > > >
> > > > At the risk of bike shedding, the sort function of Vim, when operating
> > > > with the C locale, gives a slightly different order, as experssed by
> > > > this incremental diff.
> > > >
> > > > I have no objections to your oder, but I'm slightly curious as
> > > > to how it came about.
> > >
> > > !sort which is external command.
> > >
> > > $ locale -k LC_COLLATE
> > > collate-nrules=4
> > > collate-rulesets=""
> > > collate-symb-hash-sizemb=1303
> > > collate-codeset="UTF-8"
> >
> > I'm unsure this change is worthy. It will make any later fix touching
> > the header list more difficult to backport, and I don't see a great
> > direct advantage.
>
> As Rasmus put it here
> https://lore.kernel.org/lkml/[email protected]/
> In short term you can argue that it's not beneficial, but in long term it's given
> less conflicts.
>
> > Please repost the first patch standalone.
>
> Why to repost, what did I miss? It's available via lore, just run
>
> b4 am -slt -P _ [email protected]
>
> to get it :-)

It's fairly better if actions (changes) on patches are taken by the
submitter: it scales way better, and if the other path take places we
can be easily flooded with small (but likely increasingly less smaller)
requests that will soon prevent any other activity from being taken.

Please, repost the single patch, it would be easier to me.

Thanks!

Paolo

2023-09-13 17:25:07

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

On Tue, Sep 12, 2023 at 07:25:48PM +0200, Paolo Abeni wrote:
> On Tue, 2023-09-12 at 20:04 +0300, Andy Shevchenko wrote:
> > On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote:
> > > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote:

...

> > > Please repost the first patch standalone.
> >
> > Why to repost, what did I miss? It's available via lore, just run
> >
> > b4 am -slt -P _ [email protected]
> >
> > to get it :-)
>
> It's fairly better if actions (changes) on patches are taken by the
> submitter: it scales way better, and if the other path take places we
> can be easily flooded with small (but likely increasingly less smaller)
> requests that will soon prevent any other activity from being taken.
>
> Please, repost the single patch, it would be easier to me.

Done.

--
With Best Regards,
Andy Shevchenko


2023-09-14 01:31:18

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH net-next v1 2/2] net: core: Sort headers alphabetically

On Tue, Sep 12, 2023 at 10:05 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Tue, Sep 12, 2023 at 06:53:23PM +0200, Paolo Abeni wrote:
> > On Tue, 2023-09-12 at 19:35 +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 12, 2023 at 05:20:31PM +0200, Simon Horman wrote:
> > > > On Mon, Sep 11, 2023 at 06:45:34PM +0300, Andy Shevchenko wrote:
> > > > > It's rather a gigantic list of heards that is very hard to follow.
> > > > > Sorting helps to see what's already included and what's not.
> > > > > It improves a maintainability in a long term.
> > > > >
> > > > > Signed-off-by: Andy Shevchenko <[email protected]>
> > > >
> > > > Hi Andy,
> > > >
> > > > At the risk of bike shedding, the sort function of Vim, when operating
> > > > with the C locale, gives a slightly different order, as experssed by
> > > > this incremental diff.
> > > >
> > > > I have no objections to your oder, but I'm slightly curious as
> > > > to how it came about.
> > >
> > > !sort which is external command.
> > >
> > > $ locale -k LC_COLLATE
> > > collate-nrules=4
> > > collate-rulesets=""
> > > collate-symb-hash-sizemb=1303
> > > collate-codeset="UTF-8"
> >
> > I'm unsure this change is worthy. It will make any later fix touching
> > the header list more difficult to backport, and I don't see a great
> > direct advantage.
>
> As Rasmus put it here
> https://lore.kernel.org/lkml/[email protected]/
> In short term you can argue that it's not beneficial, but in long term it's given
> less conflicts.

I agree with Paolo.

This is just code churn.
The includes will become unsorted eventually.
Headers might get renamed, split, etc.
Keeping things sorted is a headache.