Commit messages in a series of patches
FMDF
fmdefrancesco at gmail.com
Fri Sep 17 05:12:45 EDT 2021
Hi all,
I've sent a series of 19 patches (co-developed by Pavel Skripkin). Now
it is v7 and we hope it is the latest revision. :)
Few days ago, when it was still at v5, Dan Carpenter reviewed the
whole series and asked us to remove "irrelevant" information from the
commit message of 15/19: [1]
"This relates to the discussion we had about reviewing patches one at a time
in the order they arrive. Every patch should be self contained. It
should not refer to the past except in the case of explaining the Fixes
tag and it should not refer to the future except in the case where it
needs to excuse adding unused infrastructure. Reviewing is stateless.
We don't want to know about your plans.".
I decided that he was right and so I removed that extra information
(they were about plans to remove the function that I'm cleaning up in
the next patch 18/19.
The clean-ups were somewhat necessary because part of the code of the
old function is re-used in new functions introduced in 17/19 and
18/19 and it is then deleted forever, with a strange side effect that
at least one reviewer (David Laight) thought that we were renaming
variables and that renamings should go to separate patches.
However they were _not_ renamings (as I explain above). I thought that
preventive real renaming of the variables that I reuse in the next
patches within _new_ functions would have helped other developers to
review the patches 16-19/19 while avoiding that at a quick read they
could appear as renaming. Actually then David wrote that now the
patches are more easily readable.
In the meantime, Dan C. granted his "Reviewed-by" tag to the first 14
patches of 19. All patches from 2/19 to 12/19 have the following
phrase in the commit messages: "This patch is in preparation for the
_io_ops structure removal.". [2]
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.
I'm not really interested in this specific case. I'd like to have
general guidelines that I can understand and use for my future
patches.
Sorry for this long email, I wasn't able to make it shorter :(
Thanks in advance to whoever has more experience than I have and wants
to make this topic clearer to me.
Fabio
[1] https://lore.kernel.org/lkml/20210915135301.GF2088@kadam/
[2] https://lore.kernel.org/lkml/20210915124149.27543-3-fmdefrancesco@gmail.com/
More information about the Kernelnewbies
mailing list