All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <cel@kernel.org>
To: NeilBrown <neil@brown.name>
Cc: Jeff Layton <jlayton@kernel.org>,
	Olga Kornievskaia <okorniev@redhat.com>,
	Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>,
	linux-nfs@vger.kernel.org, Chuck Lever <chuck.lever@oracle.com>,
	"Darrick J. Wong" <djwong@kernel.org>,
	Luis Chamberlain <mcgrof@kernel.org>
Subject: Re: [RFC PATCH] NFSD: Add a subsystem policy document
Date: Mon, 22 Sep 2025 10:29:11 -0400	[thread overview]
Message-ID: <0fbaef6f-80ad-4885-ba2b-6a9567f01042@kernel.org> (raw)
In-Reply-To: <175851511014.1696783.3027085648108996983@noble.neil.brown.name>

Hi Neil -

On 9/21/25 9:25 PM, NeilBrown wrote:
>> +Patch preparation
>> +~~~~~~~~~~~~~~~~~
>> +Like all kernel submissions, please use tagging to identify all
>> +patch authors. Reviewers and testers can be added by replying to
>> +the email patch submission. Email is extensively used in order to
>> +publicly archive review and testing attributions, and will be
>> +automatically inserted into your patches when they are applied.
>> +
>> +The patch description must contain information that does not appear
>> +in the body of the diff. The code should be good enough to tell a
>> +story -- self-documenting -- but the patch description needs to
>> +provide rationale ("why does NFSD benefit from this change?") or
>> +a clear problem statement ("what is this patch trying to fix?").

> These paras look to be completely generic - not at all nfsd-specific.
> Do they belong here?

Can you clarify which paragraphs you mean, exactly? Maybe the whole
section?

For context:

IMHO these comments aren't necessarily generic because I haven't seen
them in other documents, and we seem to get a lot of patches where the
description is just "Make this change".

The comments about tagging: I think other subsystems might not mind
seeing Cc: stable in the initial submission. NFS maintainers (even on
the client side) like to add those themselves.

I'd like to encourage contributors to get the Fixes: tag right before
submitting, too. It saves me a little incremental time per patch.

And, some of this text was cribbed from netdev's policy document, not
from a generic document, suggesting these are subsystem addendums.


> I expect more of a patch description than is given here.  I agree that
> "code should be good enough to tell a store" but I don't think that a
> patch can by itself be good enough.
> So I think that a patch description should describe the patch -
> particularly how the various changes in the patch relate.
> 
> With a good patch description, I should be able to then read the patch
> and every change will make sense in the context provided by the
> description.  It isn't just "Why", it is also "how".

I can add these remarks.


-- 
Chuck Lever

  reply	other threads:[~2025-09-22 14:29 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-21 19:43 [RFC PATCH] NFSD: Add a subsystem policy document Chuck Lever
2025-09-22  4:25 ` NeilBrown
2025-09-22 14:29   ` Chuck Lever [this message]
2025-09-24  0:50     ` Darrick J. Wong
2025-09-24  8:48     ` NeilBrown
2025-09-24 14:07       ` Chuck Lever
2025-09-24 23:02         ` NeilBrown
2025-09-22 10:25 ` Jeff Layton
2025-09-22 13:56   ` Chuck Lever
2025-09-24  0:44     ` Darrick J. Wong
2025-09-24 14:21       ` Chuck Lever

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=0fbaef6f-80ad-4885-ba2b-6a9567f01042@kernel.org \
    --to=cel@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=dai.ngo@oracle.com \
    --cc=djwong@kernel.org \
    --cc=jlayton@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=neil@brown.name \
    --cc=okorniev@redhat.com \
    --cc=tom@talpey.com \
    /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.