[v2,3/3] docs: submit-checklist: change to autonumbered lists

Message ID 20240229030743.9125-4-lukas.bulwahn@gmail.com
State New
Headers
Series docs: submit-checklist: structure by category |

Commit Message

Lukas Bulwahn Feb. 29, 2024, 3:07 a.m. UTC
  During review (see Link), Jani Nikula suggested to use autonumbered
lists instead of manually-maintained bullet numbering.

Turn all lists into autonumbered lists.

Link: https://lore.kernel.org/linux-doc/87o7c3mlwb.fsf@intel.com/
Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
 Documentation/process/submit-checklist.rst | 48 +++++++++++-----------
 1 file changed, 24 insertions(+), 24 deletions(-)
  

Comments

Randy Dunlap Feb. 29, 2024, 6:06 a.m. UTC | #1
On 2/28/24 19:07, Lukas Bulwahn wrote:
> During review (see Link), Jani Nikula suggested to use autonumbered
> lists instead of manually-maintained bullet numbering.
> 
> Turn all lists into autonumbered lists.
> 
> Link: https://lore.kernel.org/linux-doc/87o7c3mlwb.fsf@intel.com/
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
>  Documentation/process/submit-checklist.rst | 48 +++++++++++-----------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 

Reviewed-by: Randy Dunlap <rdunlap@infradead.org>

thanks.
  
Akira Yokosawa Feb. 29, 2024, 7:52 a.m. UTC | #2
Hi Lukas,

I might be nitpicking too much, but let me go ahead...

On Thu, 29 Feb 2024 04:07:43 +0100, Lukas Bulwahn wrote:
> During review (see Link), Jani Nikula suggested to use autonumbered
> lists instead of manually-maintained bullet numbering.
> 
> Turn all lists into autonumbered lists.
> 
> Link: https://lore.kernel.org/linux-doc/87o7c3mlwb.fsf@intel.com/
> Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
> ---
>  Documentation/process/submit-checklist.rst | 48 +++++++++++-----------
>  1 file changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
> index e531dd504b6c..c984b747a755 100644
> --- a/Documentation/process/submit-checklist.rst
> +++ b/Documentation/process/submit-checklist.rst
> @@ -14,62 +14,62 @@ and elsewhere regarding submitting Linux kernel patches.
>  Review your code
>  ================
>  
> -1) If you use a facility then #include the file that defines/declares
> +#. If you use a facility then #include the file that defines/declares
>     that facility.  Don't depend on other header files pulling in ones
>     that you use.

Wait.  This will render the list starting from:

    1. If you use ...

In patch 1/1, you didn't change the ")".

It was Jani who suggested "#.", but "#)" would work just fine.

For details, see docutils' documentation at:

    https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#enumerated-lists

By the way, you should be able to use auto enumeration in the 2nd-level
list as well.

> @@ -77,7 +77,7 @@ Check your code with tools
>  Build your code
>  ===============
>  
> -1) Builds cleanly:
> +#. Builds cleanly:
>  
>    a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
>       ``=n``.  No ``gcc`` warnings/errors, no linker warnings/errors.

While the first item needs "a)", subsequent items can use "#)".

Either way,

Reviewed-by: Akira Yokosawa <akiyks@gmail.com>

        Thanks, Akira
  
Jonathan Corbet March 3, 2024, 3:55 p.m. UTC | #3
Akira Yokosawa <akiyks@gmail.com> writes:

>> -1) If you use a facility then #include the file that defines/declares
>> +#. If you use a facility then #include the file that defines/declares
>>     that facility.  Don't depend on other header files pulling in ones
>>     that you use.
>
> Wait.  This will render the list starting from:
>
>     1. If you use ...
>
> In patch 1/1, you didn't change the ")".
>
> It was Jani who suggested "#.", but "#)" would work just fine.

So I'm a little confused.  Is the objection that it renders the number
as "1." rather than "1)"?  That doesn't seem like the biggest of deals,
somehow, but am I missing something?

A bigger complaint I might raise is that auto-numbering restarts the
enumeration in each subsection, so we have a lot of steps #1, which is a
definite change from before.

That, of course, can be fixed by giving an explicit starting number in
each subsection, partially defeating the point of the change in the
first place.

I honestly have to wonder: does this document need the enumerated list
at all?  We don't refer to the numbers anywhere, so I don't think there
is much useful information there.  How about just using regular bulleted
lists instead?

That said, I don't have strong feelings one way or the other, and can
certainly apply it as-is if that's the consensus on what we should do.

Thanks,

jon
  
Randy Dunlap March 3, 2024, 6:19 p.m. UTC | #4
On 3/3/24 07:55, Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>>> -1) If you use a facility then #include the file that defines/declares
>>> +#. If you use a facility then #include the file that defines/declares
>>>     that facility.  Don't depend on other header files pulling in ones
>>>     that you use.
>>
>> Wait.  This will render the list starting from:
>>
>>     1. If you use ...
>>

I have already said that Stephen Rothwell wanted this #1 item to be at the
top of the checklist. That makes it easy to tell people to "see submit-checklist
item #1".


>> In patch 1/1, you didn't change the ")".
>>
>> It was Jani who suggested "#.", but "#)" would work just fine.
> 
> So I'm a little confused.  Is the objection that it renders the number
> as "1." rather than "1)"?  That doesn't seem like the biggest of deals,
> somehow, but am I missing something?
> 
> A bigger complaint I might raise is that auto-numbering restarts the
> enumeration in each subsection, so we have a lot of steps #1, which is a
> definite change from before.

ack

> That, of course, can be fixed by giving an explicit starting number in
> each subsection, partially defeating the point of the change in the
> first place.

ack

> I honestly have to wonder: does this document need the enumerated list
> at all?  We don't refer to the numbers anywhere, so I don't think there
> is much useful information there.  How about just using regular bulleted
> lists instead?

That also works.

> That said, I don't have strong feelings one way or the other, and can
> certainly apply it as-is if that's the consensus on what we should do.

My preference is to leave the submit-checklist numbered from 1 to N,
without a repeated #1 in each section. But I'm not hung up on it.

thanks.
  
Akira Yokosawa March 4, 2024, 1:14 a.m. UTC | #5
On Sun, 03 Mar 2024 08:55:51 -0700, Jonathan Corbet wrote:
> Akira Yokosawa <akiyks@gmail.com> writes:
> 
>>> -1) If you use a facility then #include the file that defines/declares
>>> +#. If you use a facility then #include the file that defines/declares
>>>     that facility.  Don't depend on other header files pulling in ones
>>>     that you use.
>>
>> Wait.  This will render the list starting from:
>>
>>     1. If you use ...
>>
>> In patch 1/1, you didn't change the ")".
>>
>> It was Jani who suggested "#.", but "#)" would work just fine.
> 
> So I'm a little confused.  Is the objection that it renders the number
> as "1." rather than "1)"?  That doesn't seem like the biggest of deals,
> somehow, but am I missing something?
> 

I said at the top of my reply:

> I might be nitpicking too much, but let me go ahead...

, and my point here was to let Lukas aware of the variation of auto-
numbering patterns.  I don't object the change from "1." to "1)" if
that change is intended and explained in the changelog.

HTML builder recognizes "1)", but renders it as "1.", while
LATEX builder renders it as "1)".

> A bigger complaint I might raise is that auto-numbering restarts the
> enumeration in each subsection, so we have a lot of steps #1, which is a
> definite change from before.

+1

> That, of course, can be fixed by giving an explicit starting number in
> each subsection, partially defeating the point of the change in the
> first place.

Right.

> 
> I honestly have to wonder: does this document need the enumerated list
> at all?  We don't refer to the numbers anywhere, so I don't think there
> is much useful information there.  How about just using regular bulleted
> lists instead?

That would make a lot of sense.
Auto-numbered enumerated lists look mostly the same as bulleted lists
in the source.

No strong opinion, but I'd respect Randy's preference of not applying
this change.

        Thanks, Akira

> 
> That said, I don't have strong feelings one way or the other, and can
> certainly apply it as-is if that's the consensus on what we should do.
> 
> Thanks,
> 
> jon
  

Patch

diff --git a/Documentation/process/submit-checklist.rst b/Documentation/process/submit-checklist.rst
index e531dd504b6c..c984b747a755 100644
--- a/Documentation/process/submit-checklist.rst
+++ b/Documentation/process/submit-checklist.rst
@@ -14,62 +14,62 @@  and elsewhere regarding submitting Linux kernel patches.
 Review your code
 ================
 
-1) If you use a facility then #include the file that defines/declares
+#. If you use a facility then #include the file that defines/declares
    that facility.  Don't depend on other header files pulling in ones
    that you use.
 
-2) Check your patch for general style as detailed in
+#. Check your patch for general style as detailed in
    :ref:`Documentation/process/coding-style.rst <codingstyle>`.
 
-3) All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
+#. All memory barriers {e.g., ``barrier()``, ``rmb()``, ``wmb()``} need a
    comment in the source code that explains the logic of what they are doing
    and why.
 
 Review Kconfig changes
 ======================
 
-1) Any new or modified ``CONFIG`` options do not muck up the config menu and
+#. Any new or modified ``CONFIG`` options do not muck up the config menu and
    default to off unless they meet the exception criteria documented in
    ``Documentation/kbuild/kconfig-language.rst`` Menu attributes: default value.
 
-2) All new ``Kconfig`` options have help text.
+#. All new ``Kconfig`` options have help text.
 
-3) Has been carefully reviewed with respect to relevant ``Kconfig``
+#. Has been carefully reviewed with respect to relevant ``Kconfig``
    combinations.  This is very hard to get right with testing---brainpower
    pays off here.
 
 Provide documentation
 =====================
 
-1) Include :ref:`kernel-doc <kernel_doc>` to document global kernel APIs.
+#. Include :ref:`kernel-doc <kernel_doc>` to document global kernel APIs.
    (Not required for static functions, but OK there also.)
 
-2) All new ``/proc`` entries are documented under ``Documentation/``
+#. All new ``/proc`` entries are documented under ``Documentation/``
 
-3) All new kernel boot parameters are documented in
+#. All new kernel boot parameters are documented in
    ``Documentation/admin-guide/kernel-parameters.rst``.
 
-4) All new module parameters are documented with ``MODULE_PARM_DESC()``
+#. All new module parameters are documented with ``MODULE_PARM_DESC()``
 
-5) All new userspace interfaces are documented in ``Documentation/ABI/``.
+#. All new userspace interfaces are documented in ``Documentation/ABI/``.
    See ``Documentation/ABI/README`` for more information.
    Patches that change userspace interfaces should be CCed to
    linux-api@vger.kernel.org.
 
-6) If any ioctl's are added by the patch, then also update
+#. If any ioctl's are added by the patch, then also update
    ``Documentation/userspace-api/ioctl/ioctl-number.rst``.
 
 Check your code with tools
 ==========================
 
-1) Check for trivial violations with the patch style checker prior to
+#. Check for trivial violations with the patch style checker prior to
    submission (``scripts/checkpatch.pl``).
    You should be able to justify all violations that remain in
    your patch.
 
-2) Check cleanly with sparse.
+#. Check cleanly with sparse.
 
-3) Use ``make checkstack`` and fix any problems that it finds.
+#. Use ``make checkstack`` and fix any problems that it finds.
    Note that ``checkstack`` does not point out problems explicitly,
    but any one function that uses more than 512 bytes on the stack is a
    candidate for change.
@@ -77,7 +77,7 @@  Check your code with tools
 Build your code
 ===============
 
-1) Builds cleanly:
+#. Builds cleanly:
 
   a) with applicable or modified ``CONFIG`` options ``=y``, ``=m``, and
      ``=n``.  No ``gcc`` warnings/errors, no linker warnings/errors.
@@ -90,16 +90,16 @@  Build your code
      Use ``make htmldocs`` or ``make pdfdocs`` to check the build and
      fix any issues.
 
-2) Builds on multiple CPU architectures by using local cross-compile tools
+#. Builds on multiple CPU architectures by using local cross-compile tools
    or some other build farm. Note that ppc64 is a good architecture for
    cross-compilation checking because it tends to use ``unsigned long`` for
    64-bit quantities.
 
-3) Newly-added code has been compiled with ``gcc -W`` (use
+#. Newly-added code has been compiled with ``gcc -W`` (use
    ``make KCFLAGS=-W``).  This will generate lots of noise, but is good
    for finding bugs like "warning: comparison between signed and unsigned".
 
-4) If your modified source code depends on or uses any of the kernel
+#. If your modified source code depends on or uses any of the kernel
    APIs or features that are related to the following ``Kconfig`` symbols,
    then test multiple builds with the related ``Kconfig`` symbols disabled
    and/or ``=m`` (if that option is available) [not all of these at the
@@ -112,22 +112,22 @@  Build your code
 Test your code
 ==============
 
-1) Has been tested with ``CONFIG_PREEMPT``, ``CONFIG_DEBUG_PREEMPT``,
+#. Has been tested with ``CONFIG_PREEMPT``, ``CONFIG_DEBUG_PREEMPT``,
    ``CONFIG_SLUB_DEBUG``, ``CONFIG_DEBUG_PAGEALLOC``, ``CONFIG_DEBUG_MUTEXES``,
    ``CONFIG_DEBUG_SPINLOCK``, ``CONFIG_DEBUG_ATOMIC_SLEEP``,
    ``CONFIG_PROVE_RCU`` and ``CONFIG_DEBUG_OBJECTS_RCU_HEAD`` all
    simultaneously enabled.
 
-2) Has been build- and runtime tested with and without ``CONFIG_SMP`` and
+#. Has been build- and runtime tested with and without ``CONFIG_SMP`` and
    ``CONFIG_PREEMPT.``
 
-3) All codepaths have been exercised with all lockdep features enabled.
+#. All codepaths have been exercised with all lockdep features enabled.
 
-4) Has been checked with injection of at least slab and page-allocation
+#. Has been checked with injection of at least slab and page-allocation
    failures.  See ``Documentation/fault-injection/``.
    If the new code is substantial, addition of subsystem-specific fault
    injection might be appropriate.
 
-5) Tested with the most recent tag of linux-next to make sure that it still
+#. Tested with the most recent tag of linux-next to make sure that it still
    works with all of the other queued patches and various changes in the VM,
    VFS, and other subsystems.