Message-ID: <CABVgOSko6kgA_T3LNgTPxQZS8Ab8E+XhMcOGHFx76nd2HN_RBg@mail.gmail.com>
Date:   Fri, 30 Sep 2022 18:32:16 +0800
From:   David Gow <davidgow@...gle.com>
To:     Bagas Sanjaya <bagasdotme@...il.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 Fri, Sep 30, 2022 at 5:51 PM Bagas Sanjaya <bagasdotme@...il.com> wrote:
>
> 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?
>

Yeah, that should be fine. I'm wavering back and forth on whether we
should call this a driver, given that in a lot of ways it isn't one,
but given it's in drivers/misc, it shouldn't be a problem.

> >>
> >> -.. 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.
>

I don't see any such warnings on my machine (which claims to have
sphinx-build 4.5.0).

Could you send an example warning, and your sphinx version to me so I
can try to reproduce it.

Regardless, if it's causing warnings, keep these changes. (Though it'd
be nice to include the warnings in the commit message, so it's obvious
that these are being re-aligned for a reason.)

> >>
> >>         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
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@...glegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/464981b6-d9d7-e656-261f-ef48661deaa2%40gmail.com.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4003 bytes)

Powered by blists - more mailing lists