linux-kernel - Re: [PATCH v2] Documentation: kunit: rewrite writing first test instructions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <464981b6-d9d7-e656-261f-ef48661deaa2@gmail.com>
Date:   Fri, 30 Sep 2022 16:51:08 +0700
From:   Bagas Sanjaya <bagasdotme@...il.com>
To:     David Gow <davidgow@...gle.com>
Cc:     "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        KUnit Development <kunit-dev@...glegroups.com>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Brendan Higgins <brendan.higgins@...ux.dev>,
        Jonathan Corbet <corbet@....net>,
        Khalid Masum <khalid.masum.92@...il.com>,
        Sadiya Kazi <sadiyakazi@...gle.com>
Subject: Re: [PATCH v2] Documentation: kunit: rewrite writing first test
 instructions

On 9/30/22 13:42, David Gow wrote:
> 
> While I like the idea behind this, the wording probably needs a bit of
> tweaking. In addition, by describing everything in too much detail, I
> fear we might just be adding some needless redundancy. My suspicion is
> that everyone who's going to be writing KUnit tests already knows C
> (or has access to better learning materials than this), so we're
> unlikely to need to describe in detail that, e.g., misc_example_add()
> adds two numbers together when the code is right there,
> 

We should just say "First, write the driver implementation" (without
describing writing C code in detail), right?

>>
>> -.. code-block:: c
>> +   .. code-block:: c
> 
> Why are all of these code-block declarations being indented? It
> doesn't seem to affect the actual documentation build, so I guess it's
> harmless, but it'd be better not to have it change unnecessarily and
> clutter up the diff.
> 

The indentation for code-block directive is required, since the preceding
paragraph is multiline; otherwise there will be Sphinx warnings.

>>
>>         int misc_example_add(int left, int right);
>>
>> -2. Create a file ``drivers/misc/example.c``, which includes:
>> +   Then implement the function in ``drivers/misc/example.c``:
> 
>>
>> -.. code-block:: c
>> +   .. code-block:: c
> 
> Again, code-block indentation?

Yes, for consistency.

> 
>>
>>         #include <linux/errno.h>
>>
>> @@ -152,24 +154,25 @@ In your kernel repository, let's add some code that we can test.
>>                 return left + right;
>>         }
>>
>> -3. Add the following lines to ``drivers/misc/Kconfig``:
>> +2. Add Kconfig menu entry for the feature to ``drivers/misc/Kconfig``:
> 
> This needs rewording to add back an article ("a" or "the"), and we
> probably want to call this a "Kconfig entry" rather than a "Kconfig
> menu entry". Maybe "Add a Kconfig entry for the driver..."?
> 
>>
>> -.. code-block:: kconfig
>> +   .. code-block:: kconfig
> 
> Indentation again?

Yes, see above reply.

> 
>>
>>         config MISC_EXAMPLE
>>                 bool "My example"
>>
>> -4. Add the following lines to ``drivers/misc/Makefile``:
>> +3. Add the kbuild goal that will build the feature to
>> +   ``drivers/misc/Makefile``:
> 
> Kbuild goal? I've never heard of this being called a Kbuild goal before?
> 
> How about a "make target"?
> 

At the time of writing this patch, I use terminology in
Documentation/kbuild/makefiles.rst, which the "make target" is called
"Kbuild goal".

>>
>> -.. code-block:: make
>> +   .. code-block:: make
> 
> Indentation?

Yes, for consistency with the first code-block directive.

>>
>> -.. code-block:: c
>> +   .. code-block:: c
> 
> Indentation.
> 

See above reply.

>>
>> -.. code-block:: kconfig
>> +   .. code-block:: kconfig
> 
> Indentation?

See above reply.

>>
>> -.. code-block:: make
>> +   .. code-block:: make
> 
> Indentation?

See above reply.

>>
>> -.. code-block:: none
>> +   .. code-block:: none
> 
> Indentation?
> 

See above reply.

>>
>>         CONFIG_MISC_EXAMPLE=y
>>         CONFIG_MISC_EXAMPLE_TEST=y
>>
>>  5. Run the test:
>>
>> -.. code-block:: bash
>> +   .. code-block:: bash
> 
> Indentation?

See above reply.

Thanks for reviewing.

-- 
An old man doll... just what I always wanted! - Clara