On Thu, Apr 9, 2020 at 11:16 PM Brendan Higgins
<brendanhiggins@xxxxxxxxxx> wrote:
>
> On Thu, Apr 9, 2020 at 1:51 PM Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:
> >
> > On Thu, Apr 9, 2020 at 10:00 PM Brendan Higgins
> > <brendanhiggins@xxxxxxxxxx> wrote:
> > >
> > > On Wed, Apr 8, 2020 at 2:17 PM Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:
> > > >
> > > > On Wed, Apr 8, 2020 at 10:56 PM Brendan Higgins
> > > > <brendanhiggins@xxxxxxxxxx> wrote:
> > > > >
> > > > > On Wed, Apr 8, 2020 at 1:50 PM Lothar Rubusch <l.rubusch@xxxxxxxxx> wrote:
>
> [...]
>
> > > > > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > > > > index 9b0c46a6ca1f..16d548b795b5 100644
> > > > > > --- a/include/kunit/test.h
> > > > > > +++ b/include/kunit/test.h
> > > > > > @@ -175,7 +175,7 @@ struct kunit_suite {
> > > > > > void (*exit)(struct kunit *test);
> > > > > > struct kunit_case *test_cases;
> > > > > >
> > > > > > - /* private - internal use only */
> > > > > > + /* private: internal use only */
> > > > > > struct dentry *debugfs;
> > > > > > char *log;
> > > > > > };
> > > > > > @@ -232,7 +232,8 @@ void __kunit_test_suites_exit(struct kunit_suite **suites);
> > > > > > * kunit_test_suites() - used to register one or more &struct kunit_suite
> > > > > > * with KUnit.
> > > > > > *
> > > > > > - * @suites: a statically allocated list of &struct kunit_suite.
> > > > > > + * @...: a statically allocated list of &struct kunit_suite, assigned
> > > > > > + * to the pointer @suites.
> > > > > > *
> > > > > > * Registers @suites with the test framework. See &struct kunit_suite for
> > > > >
> > > > > Can you change the @suites param here to match @...?
> > > >
> > > > You mean, in "Registers @suites with the test framework" to something
> > > > rather like "Registers @... with"?
> > > > Hum, franckly I think, in the documentation it reads better having the
> > > > name "suites", that's why I tried to
> > > > append a hint, that a passed list of struct kunit_suite initializes
> > > > the pointer "suites". Then further in the doc
> > > > refered as suites, I think it becomes clear. But let me know. Shall I
> > > > use @... instead?
> > >
> > > I agree that it doesn't read as well, but I like having the proper
> > > syntax highlighting and consistent naming over a mix and match.
> >
> > I'll have another look on the syntax highlighting, leave out the api page
> > removal and then resubmit. No problem. Thank you for all your patience!!
> > I really appreciate if I can contribute to something!
> >
> >
> > > Another alternative would be to replace `...` with `suites...` and
> > > then @suites should work.
> > >
> > > Either way is fine with me; it's a nasty macro anyway.
> >
> > Yeah, that's what I read in the kernel-doc perl, too. Changing '...'
> > to 'suites...'
> > is a code change, though. Possible, but IMHO would be a different patch.
>
> Good thought; that would usually be true most likely; however, in this
> case the documentation issue is in a comment in a code file, so no
> matter how you look at it, it is a code change. Also, it's such a
> minor change and since most of the KUnit code and documentation
> changes both go through the KUnit branch of the Kselftest tree anyway,
> so I think it's fine to just keep it all in one patch.
Absolutely Agree, this is cosmetics. Anyway, personally I'm also learning how
to write to the community, i.e. I try to be careful. Thanks for the support!
> > Actually, should we fix up the code for having a nicer documentation?!!
>
> Yes! Or at least I think so. Good documentation can be just as
> important, even more important than good code. I know there are plenty
> of developers that disagree with me on this point, but given what we
> are trying to do with KUnit, I say we are actually one of the places
> where our documentation is of utmost importance.
Ah, this is actually not what I meant. I fully agree with you, good generated
documentation is essential. I wanted to say, In a way I would prefer changing
the scripts, rather than the sources to obtain better documentation.
Practically it's easier to adjust the source directly.
> Still, great question to ask. Different kernel developers feel very
> differently about this point, and with good reason: some parts of the
> kernel will only really be used by people who can be expected to read
> all the relevant code, whereas other parts of the kernel may be used
> directly by a huge number of people most of which can't be expected to
> read all the code. It would make sense that these two groups of people
> would find different value in documentation.
>
> > Somehow this feels like the next patches should go rather to sphinx/kernel-doc.
>
> Not sure what you mean by this.
Generally I see the limitation in the kernel-doc scripts not allowing
for an anotation
for a better naming an unnamed variadics. I mean changing the source
according to
the limitations of the scripts, is still kind of reasonable, low prio,
low effort, easy...
Alternatively, extending the scripts may add more flexibility.
I saw worse things at the sphinx documentation e.g. duplicate label
warnings when
using same headers inside the same document and sphinx.ext.autosection label
active - which makes me feel like this should be rather patched, than
rewritten in
the docs to fit that quirk of sphinx. Getting offtopic..
> > Anyway, this patch is all about documentation.
> > A code change must be tested and verified and IMHO might be a different story.
>
> True, all code changes should be tested (I think all docs changes
> should be tested too :-) ), but testing this code here is really easy.
> You can test it with the following command:
>
> ./tools/testing/kunit/kunit.py run --timeout=60 --jobs=8 --defconfig
This is just awesome. Yesterday I was too lazy to still try anything.
Thank you, I appreciate. I'll play around with that the next days.
> >
> > Happy Easter!
>
> Happy Easter! (again)
L
