Re: [RFC PATCH v5 0/7] mseal system mappings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* Kees Cook <kees@xxxxxxxxxx> [250213 14:34]:
> 
> 
> On February 13, 2025 10:35:21 AM PST, "Liam R. Howlett" <Liam.Howlett@xxxxxxxxxx> wrote:
> >* Kees Cook <kees@xxxxxxxxxx> [250212 17:05]:
> >> On Wed, Feb 12, 2025 at 11:24:35AM +0000, Lorenzo Stoakes wrote:
> >> > On Wed, Feb 12, 2025 at 03:21:48AM +0000, jeffxu@xxxxxxxxxxxx wrote:
> >> > > From: Jeff Xu <jeffxu@xxxxxxxxxxxx>
> >> > >
> >> > > The commit message in the first patch contains the full description of
> >> > > this series.
> >> > 
> >> > Sorry to nit, but it'd be useful to reproduce in the cover letter too! But
> >> > this obviously isn't urgent, just be nice when we un-RFC.
> >> 
> >> I advised Jeff against this because I've found it can sometimes cause
> >> "thread splitting" in that some people reply to the cover letter, and
> >> some people reply to the first patch, etc. I've tended to try to keep
> >> cover letters very general, with the bulk of the prose in the first
> >> patch.
> >
> >Interesting idea, but I think thread splitting is less of a concern than
> >diluting the meaning of a patch by including a lengthy change log with a
> >fraction of the text being about the code that follows.
> >
> >I think this is the reason for a cover letter in the first place; not
> >just version control.  After all, we could tack the version information
> >into the first patch too and avoid it being in the final commit message.
> 
> Okay, so to be clear: you'd prefer to put the rationales and other stuff in the cover, and put more specific details in the first patch? I've not liked this because cover letters aren't (except for akpm's trees) included anywhere in git, which makes archeology much harder.

Yes, rationales in the cover letter.  I like the way the akpm's tree
does things because it's the best of both worlds.  There is also a
separation of the cover letter with the actual commit message on the
first patch.

Having the full cover letter on patch 1 makes it difficult to understand
*that* patch on its own during review.

I've also gotten emails from Linus asking why in the ____ing ____ I did
it this way when I said why in the cover letter.. to that note I like
the patches to have _all_ the necessary details for that one patch,
including the sometimes "this is changed in the very next patch" lines
to spell out in-transit patches, or what ever else is needed from the
cover letter/context.

Taking this example, we have a 111 line cover letter and a patch that
adds a new file with a single function, and two kconfig options.  The
justification and reason for the patch is in the middle of that huge
block of text.  That seems silly.

That is to say:
Cover letters have the rationale and over-arching reason.
Patches have more than enough details to code inspect and know why this
patch is necessary.

Thanks,
Liam




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux