On Thu, Apr 9, 2020 at 11:16 PM Brendan Higgins brendanhiggins@google.com wrote:
On Thu, Apr 9, 2020 at 1:51 PM Lothar Rubusch l.rubusch@gmail.com wrote:
On Thu, Apr 9, 2020 at 10:00 PM Brendan Higgins brendanhiggins@google.com wrote:
On Wed, Apr 8, 2020 at 2:17 PM Lothar Rubusch l.rubusch@gmail.com wrote:
On Wed, Apr 8, 2020 at 10:56 PM Brendan Higgins brendanhiggins@google.com wrote:
On Wed, Apr 8, 2020 at 1:50 PM Lothar Rubusch l.rubusch@gmail.com 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