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
- Run the test:
-.. code-block:: bash
- .. code-block:: bash
Indentation?
See above reply.
Thanks for reviewing.