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.
> 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.
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.
> 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
>
> Happy Easter!
Happy Easter!
