[PATCH] Documentation: test.h - fix warnings

[PATCH] Documentation: test.h - fix warnings

[Lothar Rubusch]

Fix several sphinx warnings at 'make htmldocs'
- privately declared members not correctly declared as such
- 'suits' actually is not a function parameter, change declaration to fix
  warning but keep information in comment

Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
 include/kunit/test.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h
index 9b0c46a6ca1f..fe4ea388528b 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,7 @@ 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.
+ * suites - a statically allocated list of &struct kunit_suite.
  *
  * Registers @suites with the test framework. See &struct kunit_suite for
  * more information.
-- 
2.20.1

Re: [PATCH] Documentation: test.h - fix warnings

[Brendan Higgins]

On Mon, Apr 6, 2020 at 2:41 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Fix several sphinx warnings at 'make htmldocs'
> - privately declared members not correctly declared as such
> - 'suits' actually is not a function parameter, change declaration to fix
>   warning but keep information in comment
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>

Thanks for taking care of this!

> ---
>  include/kunit/test.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 9b0c46a6ca1f..fe4ea388528b 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,7 @@ 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.
> + * suites - a statically allocated list of &struct kunit_suite.

So, I am pretty sure you can name the variadic arguments and then that
gives you a valid parameter to use with kernel doc. Can you try that
out?

>   *
>   * Registers @suites with the test framework. See &struct kunit_suite for

Also, if my suggestion ends up not working, you should change this
line to match.

>   * more information.
> --
> 2.20.1
>

Re: [PATCH] Documentation: test.h - fix warnings

[Lothar Rubusch]

On Tue, Apr 7, 2020 at 10:49 PM Brendan Higgins
<brendanhiggins@google.com> wrote:
>
> On Mon, Apr 6, 2020 at 2:41 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> >
> > Fix several sphinx warnings at 'make htmldocs'
> > - privately declared members not correctly declared as such
> > - 'suits' actually is not a function parameter, change declaration to fix
> >   warning but keep information in comment
> >
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
>
> Thanks for taking care of this!
>
> > ---
> >  include/kunit/test.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > index 9b0c46a6ca1f..fe4ea388528b 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,7 @@ 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.
> > + * suites - a statically allocated list of &struct kunit_suite.
>
> So, I am pretty sure you can name the variadic arguments and then that
> gives you a valid parameter to use with kernel doc. Can you try that
> out?
>
You mean the warning "Excess function parameter 'suites' description
in 'kunit_test_suites'"?
Honestly, due to the TODO in the same comment section, It seemed to me kind of a
work-in-progress situation which I didn't dare to interfere.

> >   *
> >   * Registers @suites with the test framework. See &struct kunit_suite for
>
> Also, if my suggestion ends up not working, you should change this
> line to match.
>

Sure, sounds interesting I can try to figure out. Thank you for the answer.
L

Re: [PATCH] Documentation: test.h - fix warnings

[Brendan Higgins]

On Tue, Apr 7, 2020 at 3:30 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> On Tue, Apr 7, 2020 at 10:49 PM Brendan Higgins
> <brendanhiggins@google.com> wrote:
> >
> > On Mon, Apr 6, 2020 at 2:41 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
> > >
> > > Fix several sphinx warnings at 'make htmldocs'
> > > - privately declared members not correctly declared as such
> > > - 'suits' actually is not a function parameter, change declaration to fix
> > >   warning but keep information in comment
> > >
> > > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> >
> > Thanks for taking care of this!
> >
> > > ---
> > >  include/kunit/test.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/kunit/test.h b/include/kunit/test.h
> > > index 9b0c46a6ca1f..fe4ea388528b 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,7 @@ 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.
> > > + * suites - a statically allocated list of &struct kunit_suite.
> >
> > So, I am pretty sure you can name the variadic arguments and then that
> > gives you a valid parameter to use with kernel doc. Can you try that
> > out?
> >
> You mean the warning "Excess function parameter 'suites' description
> in 'kunit_test_suites'"?

Yep, I just tried it out locally and it should work.

> Honestly, due to the TODO in the same comment section, It seemed to me kind of a
> work-in-progress situation which I didn't dare to interfere.

Don't worry about that. Addressing that TODO is going to take some time.

> > >   *
> > >   * Registers @suites with the test framework. See &struct kunit_suite for
> >
> > Also, if my suggestion ends up not working, you should change this
> > line to match.
> >
>
> Sure, sounds interesting I can try to figure out. Thank you for the answer.

Thanks!

Re: [PATCH] Documentation: test.h - fix warnings

[Lothar Rubusch]

Hello,

(...)
> > > > @@ -232,7 +232,7 @@ 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.
> > > > + * suites - a statically allocated list of &struct kunit_suite.
> > >
> > > So, I am pretty sure you can name the variadic arguments and then that
> > > gives you a valid parameter to use with kernel doc. Can you try that
> > > out?
> > >
> > You mean the warning "Excess function parameter 'suites' description
> > in 'kunit_test_suites'"?
>
> Yep, I just tried it out locally and it should work.

Something like '@...:' should be possible, to list the variadic under
"Parameters", and display the corresponding description.

For curiosity I went through the kunit docs and fixed some formatting
issues. Great piece of work, thanks for realizing kunit!

Last but not least, going through the document, there is a chapter
"API" consisting of a single page with one link to "API".
Is this on purpose? I would suggest to take out this level of
indirection and remove the .rst file, linking API directly.

I'll send my proposals in a PATCH v2. Let me know then what you think
after that.

Best,
L

Re: [PATCH] Documentation: test.h - fix warnings

[Brendan Higgins]

On Wed, Apr 8, 2020 at 1:39 PM Lothar Rubusch <l.rubusch@gmail.com> wrote:
>
> Hello,
>
> (...)
> > > > > @@ -232,7 +232,7 @@ 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.
> > > > > + * suites - a statically allocated list of &struct kunit_suite.
> > > >
> > > > So, I am pretty sure you can name the variadic arguments and then that
> > > > gives you a valid parameter to use with kernel doc. Can you try that
> > > > out?
> > > >
> > > You mean the warning "Excess function parameter 'suites' description
> > > in 'kunit_test_suites'"?
> >
> > Yep, I just tried it out locally and it should work.
>
> Something like '@...:' should be possible, to list the variadic under
> "Parameters", and display the corresponding description.

That'll probably work, or you can change the `...` to something like
`suites_param...` and change the corresponding `__VA_ARGS__` to
`suites_param`. Either way works for me.

> For curiosity I went through the kunit docs and fixed some formatting
> issues. Great piece of work, thanks for realizing kunit!

Thanks!

> Last but not least, going through the document, there is a chapter
> "API" consisting of a single page with one link to "API".
> Is this on purpose? I would suggest to take out this level of
> indirection and remove the .rst file, linking API directly.

Yeah, it's because we have some other features that we are planning on
adding soonish which will have their own pages.

> I'll send my proposals in a PATCH v2. Let me know then what you think
> after that.

Looking forward to them!

Cheers