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
