All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Horman <horms@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>, Jonathan Corbet <corbet@lwn.net>,
	netdev@vger.kernel.org, workflows@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH RFC net] docs: netdev: document guidance on cleanup patches
Date: Mon, 7 Oct 2024 17:15:01 +0100	[thread overview]
Message-ID: <20241007161501.GJ32733@kernel.org> (raw)
In-Reply-To: <20241007090828.05c3f0da@kernel.org>

On Mon, Oct 07, 2024 at 09:08:28AM -0700, Jakub Kicinski wrote:
> On Mon, 7 Oct 2024 16:55:21 +0100 Simon Horman wrote:
> > > > +Netdev discourages patches which perform simple clean-ups, which are not in
> > > > +the context of other work. For example addressing ``checkpatch.pl``
> > > > +warnings, or :ref:`local variable ordering<rcs>` issues. This is because it
> > > > +is felt that the churn that such changes produce comes at a greater cost
> > > > +than the value of such clean-ups.  
> > > 
> > > Should we add "conversions to managed APIs"? It's not a recent thing,
> > > people do like to post patches doing bulk conversions which bring very
> > > little benefit.  
> > 
> > Well yes, I agree that is well established, and a common target of patches.
> > But isn't that covered by the previous section?
> > 
> >    "Using device-managed and cleanup.h constructs
> >     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > 
> >    "Netdev remains skeptical about promises of all “auto-cleanup” APIs,
> >     including even devm_ helpers, historically. They are not the preferred
> >     style of implementation, merely an acceptable one.
> > 
> >     ...
> > 
> >    https://docs.kernel.org/process/maintainer-netdev.html#using-device-managed-and-cleanup-h-constructs
> > 
> > We could merge or otherwise rearrange that section with the one proposed by
> > this patch. But I didn't feel it was necessary last week.
> 
> Somewhat, we don't push back on correct use of device-managed APIs.
> But converting ancient drivers to be device-managed just to save 
> 2 or 3 LoC is pointless churn. Which in my mind falls squarely
> under the new section, the new section is intended for people sending
> trivial patches.

Thanks, I can try and work with that. Do you want to call out older drivers
too? I was intentionally skipping that for now. But I do agree it should
be mentioned at some point.

> > > On the opposite side we could mention that spelling fixes are okay.
> > > Not sure if that would muddy the waters too much..  
> > 
> > I think we can and should. Perhaps another section simply stating
> > that spelling (and grammar?) fixes are welcome.
> 
> Hm, dunno, for quotability I'd have a weak preference for a single
> section describing what is and isn't acceptable as a standalone cleanup.

Sure.

  reply	other threads:[~2024-10-07 16:15 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-04  9:49 [PATCH RFC net] docs: netdev: document guidance on cleanup patches Simon Horman
2024-10-07 15:24 ` Jakub Kicinski
2024-10-07 15:55   ` Simon Horman
2024-10-07 16:08     ` Jakub Kicinski
2024-10-07 16:15       ` Simon Horman [this message]
2024-10-07 16:54         ` Jakub Kicinski
2024-10-08 12:30           ` Simon Horman

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=20241007161501.GJ32733@kernel.org \
    --to=horms@kernel.org \
    --cc=corbet@lwn.net \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=workflows@vger.kernel.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.