Re: [LTP] [PATCH 1/1] include: doc: Convert comments into linuxdoc

Cyril Hrubis
2024-03-25T12:54:36Z

Hi!
> generally LGTM, not looking deeply as you plan to send v2.
>
> Do you plan later to work also on include/tst_test_macros.h?

Maybe later, I'm not sure if I get to that part soo enough.

> Do you need any help on docs?

Proof reading is always welcomed, but please wait for v2.

> Also I should find time to study how to generate docs locally.

That is actually quite easy, you need the patchset from andrea with this
patch on the top. Then you need install sphinx and the linuxdoc tooling.
The spinx is usually packaged in your distro, the linuxdoc has to be
installed with pip as described at:

https://github.com/return42/linuxdoc

After that it's just a matter of typing make in the doc_new directory.

> ...
> > /**
> > - * Add, check or remove a capabilities
> > + * tst_cap_setup() - Add, check or remove a capabilities.
> > + *
> > + * @cap: An {} terminated array of capabilities to alter.
> > + * @action_mask: Decides which actions are done, i.e. only drop caps, add them
> > + * or both.
> > *
> > * Takes a NULL terminated array of structs which describe whether some
> > * capabilities are needed or not and mask that determines subset of the
> > * actions to be performed. Loops over the array and if mask matches the
> > * element action it's passed to tst_cap_action().
> > */
> > -void tst_cap_setup(struct tst_cap *cap, unsigned int action_mask);
> > +void tst_cap_setup(struct tst_cap *cap, enum tst_cap_act action_mask);
>
> This is a real code change, which could be merged separately on the docs change.
> But I see whole patchset is not only about docs, but about moving definitions
> into enums thus OK for me.

Some of headers need a bit of cleaning so that the documentation looks
nicer. It could be done in a separate patch if you really want to, but I
do not think that it's that important.

> > #endif /* TST_CAPABILITY_H */
> > diff --git a/include/tst_checkpoint.h b/include/tst_checkpoint.h
> > index 1b6911d73..62d87d60b 100644
> > --- a/include/tst_checkpoint.h
> > +++ b/include/tst_checkpoint.h
> > @@ -1,24 +1,86 @@
> > -/* SPDX-License-Identifier: GPL-2.0-or-later
> > +// SPDX-License-Identifier: GPL-2.0-or-later
>
> I guess this change will cause checkpatch.pl to complain:
>
> tst_checkpoint.h:1: WARNING: Improper SPDX comment style for 'tst_checkpoint.h', please use '/*' instead
> tst_checkpoint.h:1: WARNING: Missing or malformed SPDX-License-Identifier tag in line 1

Sigh, I was sure that I fixed that, will fix in v2.

--
Cyril Hrubis
chrubis@suse.cz