From: "Valdis Klētnieks" <valdis.kletnieks@vt.edu>
To: FMDF <fmdefrancesco@gmail.com>
Cc: kernelnewbies <kernelnewbies@kernelnewbies.org>
Subject: Re: Commit messages in a series of patches
Date: Fri, 17 Sep 2021 17:58:48 -0400 [thread overview]
Message-ID: <90319.1631915928@turing-police> (raw)
In-Reply-To: <CAPj211v_oKq7VEhUROiVExz8W3aTXDZO5BwyEJoyqdsp8d879w@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 914 bytes --]
On Fri, 17 Sep 2021 11:12:45 +0200, FMDF said:
> My question is: why "This patch is preparation for _io_ops [future]
> structure removal." is good while "Eventually this function will be
> deleted but some of the code will be reused later." is not.
The first is specific about what is being changed and why, and tells the
reviewer what to watch for. Also, the reviewer now knows where to look for
justification - there is hopefully a 0/N patch that explains why and how this
structure is being removed.
The second doesn't say which "this function" is being removed, why this is
being done, or whether the changes were internal to the function, or in other
functions. It also doesn't explain why or how code will be re-used.
The distinction matters, because the biggest point of reviewing is "Does this
commit do what was intended?" Answering that question is a lot easier when
it's clear what was intended.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 494 bytes --]
[-- Attachment #2: Type: text/plain, Size: 170 bytes --]
_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@kernelnewbies.org
https://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
next prev parent reply other threads:[~2021-09-17 22:04 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-17 9:12 Commit messages in a series of patches FMDF
2021-09-17 21:58 ` Valdis Klētnieks [this message]
[not found] ` <CAPj211ufxfjritLEc8n11=OLXFFjGSAMacvgOAb2usOeS0LEwg@mail.gmail.com>
2021-09-20 9:17 ` Fwd: " FMDF
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=90319.1631915928@turing-police \
--to=valdis.kletnieks@vt.edu \
--cc=fmdefrancesco@gmail.com \
--cc=kernelnewbies@kernelnewbies.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).