All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Marco Elver <elver@google.com>,
	linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com,
	linux-mm@kvack.org, Steven Rostedt <rostedt@goodmis.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Alexander Potapenko <glider@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH v2 1/3] lib/vsprintf: Sort headers alphabetically
Date: Mon, 14 Aug 2023 17:33:49 +0200	[thread overview]
Message-ID: <ZNpJXapjZcYqJqFG@alley> (raw)
In-Reply-To: <5eca0ab5-84be-2d8f-e0b3-c9fdfa961826@rasmusvillemoes.dk>

On Mon 2023-08-07 21:47:17, Rasmus Villemoes wrote:
> On 07/08/2023 16.58, Andy Shevchenko wrote:
> > On Mon, Aug 07, 2023 at 04:31:37PM +0200, Petr Mladek wrote:
> >> On Sat 2023-08-05 20:50:25, Andy Shevchenko wrote:
> >>> Sorting headers alphabetically helps locating duplicates, and
> >>> make it easier to figure out where to insert new headers.
> >>
> >> I agree that includes become a mess after some time. But I am
> >> not persuaded that sorting them alphabetically in random source
> >> files help anything.
> >>
> >> Is this part of some grand plan for the entire kernel, please?
> >> Is this outcome from some particular discussion?
> >> Will this become a well know rule checked by checkpatch.pl?
> >>
> >> I am personally not going to reject patches because of wrongly
> >> sorted headers unless there is some real plan behind it.
> >>
> >> I agree that it might look better. An inverse Christmas' tree
> >> also looks better. But it does not mean that it makes the life
> >> easier.
> > 
> > It does from my point of view as maintainability is increased.
> > 
> >> The important things are still hidden in the details
> >> (every single line).
> >>
> >> From my POV, this patch would just create a mess in the git
> >> history and complicate backporting.
> >>
> >> I am sorry but I will not accept this patch unless there
> >> is a wide consensus that this makes sense.
> > 
> > Your choice, of course, But I see in practice dup headers being
> > added, or some unrelated ones left untouched because header list
> > mess, and in those cases sorting can help (a bit) in my opinion.
> 
> I agree with Andy on this one. There doesn't need to be some grand
> master plan to apply this to the entire kernel, but doing it to
> individual files bit by bit does increase the maintainability. And I
> really don't buy the backporting argument. Sure, backporting some patch
> across the release that does the sorting is harder - but then,
> backporting the sorting patch itself is entirely trivial (maybe not the
> textual part, but redoing the semantics of it is). _However_,
> backporting a patch from release z to release y, both of which being
> later than the release x that did the sorting, is going to be _easier_.
> It also reduces merge conflicts - that's also why lots of Makefiles are
> kept sorted.

I am afraid that we will not find a consensus here. I agree that
the sorting has some advantage.

But I would still like to get some wider agreement on this move
from other subsystem. It is a good candidate for a mass change
which would be part of some plan.

I want to avoid reshuffling this more times according to personal
preferences. And I do not want to add this cosmetic subsystem
specific requirement.

Best Regards,
Petr

  reply	other threads:[~2023-08-14 15:34 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-05 17:50 [PATCH v2 0/3] lib/vsprintf: Rework header inclusions Andy Shevchenko
2023-08-05 17:50 ` [PATCH v2 1/3] lib/vsprintf: Sort headers alphabetically Andy Shevchenko
2023-08-07 14:31   ` Petr Mladek
2023-08-07 14:53     ` Sergey Senozhatsky
2023-08-07 15:00       ` Andy Shevchenko
2023-08-07 14:58     ` Andy Shevchenko
2023-08-07 19:47       ` Rasmus Villemoes
2023-08-14 15:33         ` Petr Mladek [this message]
2023-08-05 17:50 ` [PATCH v2 2/3] lib/vsprintf: Split out sprintf() and friends Andy Shevchenko
2023-08-05 18:43   ` Andrew Morton
2023-08-05 21:31     ` Andy Shevchenko
2023-08-08 12:55     ` Andy Shevchenko
2023-08-07 15:03   ` Petr Mladek
2023-08-07 15:09     ` Andy Shevchenko
2023-08-07 15:11       ` Andy Shevchenko
2023-08-07 15:13         ` Andy Shevchenko
2023-08-08  6:41           ` Petr Mladek
2023-08-08 12:47             ` Andy Shevchenko
2023-08-10  8:15               ` Petr Mladek
2023-08-10  9:09                 ` Rasmus Villemoes
2023-08-10 13:17                   ` Andy Shevchenko
2023-08-10 14:17                     ` Rasmus Villemoes
2023-08-11 19:28                       ` Steven Rostedt
2023-08-14 11:16                         ` Andy Shevchenko
2023-08-14 15:16                   ` Petr Mladek
2023-08-15  9:58                   ` David Laight
2023-08-09  8:48             ` David Laight
2023-08-10 13:13               ` Andy Shevchenko
2023-08-14  8:12                 ` David Laight
2023-08-14 12:28                   ` 'Andy Shevchenko'
2023-08-08  2:24       ` Steven Rostedt
2023-08-08 12:49         ` Andy Shevchenko
2023-08-07 19:31     ` Rasmus Villemoes
2023-08-08 11:17       ` David Laight
2023-08-05 17:50 ` [PATCH v2 3/3] lib/vsprintf: Declare no_hash_pointers in sprintf.h Andy Shevchenko
2023-08-07  6:00   ` Marco Elver
2023-08-07 15:06   ` Petr Mladek
2023-08-07 15:27     ` Andy Shevchenko
2023-08-14 15:38 ` [PATCH v2 0/3] lib/vsprintf: Rework header inclusions Petr Mladek
2023-08-14 16:11   ` Andy Shevchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZNpJXapjZcYqJqFG@alley \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@rasmusvillemoes.dk \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.