Received-SPF: pass (google.com: domain of linux-kernel+bounces-82476-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <43df625f-bd32-4dd9-a960-6d0f5c0304c7@infradead.org>
Date: Mon, 26 Feb 2024 16:41:36 -0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] docs: submit-checklist: structure by category
Content-Language: en-US
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>, Jonathan Corbet
 <corbet@lwn.net>, workflows@vger.kernel.org, linux-doc@vger.kernel.org
Cc: kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20240226104653.54877-1-lukas.bulwahn@gmail.com>
From: Randy Dunlap <rdunlap@infradead.org>
In-Reply-To: <20240226104653.54877-1-lukas.bulwahn@gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

Hi Lukas,

I'll review the file changes separately. This is just replying
to the patch description comments.


On 2/26/24 02:46, Lukas Bulwahn wrote:
> While going through the submit checklist, the list order seemed rather
> random, probably just by historical coincidences of always adding yet the
> next point someone thought of at the end of the list.

Probably.

> Structure and order them by the category of such activity,
> reviewing, documenting, checking with tools, building and testing.
> 
> As the diff of the reordering is large:
> Review code now includes previous points 1, 5 and 22.
> Review Kconfig includes previous 6, 7 and 8.
> Documenting includes previous 11, 15, 16, 17, 18 and 23.
> Checking with tools includes previous 5, 9 and 10.
> Building includes previous 2, 3, 20 and 24.
> Testing includes previous 12, 13, 14, 19 and 21.
> 
..

> 
> The recommendation to test with the -mm patchset (previous 21, now
> testing, point 5) was updated to the current state of affairs to test with
> a recent tag of linux-next.

ack.

> Note that the previous first point still remains the first list even after
> reordering. Based on some vague memory, the first point was important to
> Randy to stay the first one in any reordering.

Yes, I have said that. Stephen Rothwell wanted it to be first in the list.


> While at it, the reference to CONFIG_SLUB_DEBUG was replaced by
> CONFIG_DEBUG_SLAB.

I don't understand this comment. DEBUG_SLAB is gone.
I think those 2 symbols might be reversed in your comments. ?


> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
> So far, no point disappeared and nothing new was added.
> 

That's a good start IMO.

> Points/Ideas for further improvements (based on my knowledge and judgement):
> 
>   - The Review Kconfig changes makes sense, but I am not sure if they are
>     so central during review. If we keep it, let us see if there are other
>     parts for review that are also important similar to Kconfig changes.
>    
>   - Concerning checking with tools, checkpatch probably still makes sense;
>     it pointed out in several places. If sparse and checkstack are really
>     the next two tools to point out, I am not so sure about.

I doubt that ckeckstack is important since gcc & clang warn us about
stack usage.

>     sparse has a lot of false positives nowadays, and many things are not
>     fixed just because sparse complains about it.
>     And I have never used make checkstack and have not found much
>     documentation about it.
>     So, maybe other tools deserve to be mentioned here instead?
> 
> I am happy to get feedback---I will work through submitting-patches next
> and do some clean-up there. While doing that, I might learn what really
> should go into a better future 'submit-checklist' documentation.
> 
>  Documentation/process/submit-checklist.rst | 157 +++++++++++----------
>  1 file changed, 84 insertions(+), 73 deletions(-)


-- 
#Randy