-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Doc: fix and improve doxygen grouping of boards_common #8189
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO doc.txt
s should only be kept to a minimum. They should only be used for packages, pseudo-modules and the main modules like drivers
, sys
, etc.
boards/common/arduino-mkr/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_arduino-mkr Arduino MKR Common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why create the group here and not leave the definition in board_common.h
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it more convenient to have the @defgroup
s in doc.txt
to allow for extended description and documentation of a group, having this in the header file makes the boiler plate of that file (IMHO) rather long. Though, at the moment, there is no such description for these groups - so if yo insist, I can move that (back) to the header file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@miri64 is this blocking for you, i.e., would you rather have this changed, or can we move on (for now)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It kind of is...
having this in the header file makes the boiler plate of that file (IMHO) rather long.
Why is that a problem? I see it rather as a benefit. If the detailed doc is somewhere hidden in a doc file noone will read it (or even worse edit it on a major change). If the description however is directly attached to the code chances are higher that people will find it (especially if it is a well written long-ish documentation block) ;-).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we should clarify that later on. I still favour my approach of using doc.txt
for defining doc groups and optionally add more extensive information there.
If the detailed doc is somewhere hidden in a doc file noone will read it
I don't agree with this argument, because the doc.txt
resides in the root/main directory not in some subdirectories and their name (again: doc.txt
) should be rather tempting to look at to everyone searching for documentation, or not?
Anyway, as in this case the doc.txt
only contains the @defgroup
its simpler to have it in the header.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO (and why it was introduced in the first place) the doc.txt
approach is a workaround for modules where we do not have a file to put it in. As you might have noticed, it still uses C-syntax to let doxygen be able to read that file at all. The beauty of Doxygen is that it can reside with the code, so why not use it?!??
boards/common/frdm/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_frdm NXP FRDM Common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
boards/common/iotlab/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_iotlab IoTlab Common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
boards/common/msb-430/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_msb-430 MSB-430 common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
boards/common/msba2/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_msba2 MSB-A2 common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
boards/common/nucleo/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_nucleo STM Nucleo common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
boards/common/nucleo32/doc.txt
Outdated
*/ | ||
|
||
/** | ||
* @defgroup boards_common_nucleo32 STM Nucleo32 common |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito, etc.
c7c0054
to
108aa7d
Compare
comments addressed |
also compiled docs and checked grouping, this PR fixes wrong grouping of nrf52 board currently visible in our doc + several other groupings |
Your last commit is prefixed |
Apart from that: confirmed that doc is fixed now. |
108aa7d
to
b1df79a
Compare
oh damn ... anyway, fixed the typo |
@miri64 happy? I think we should get our doc right again 😉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, ACK
This started of with a bug in the current docu after #8058 was merged where nrf52 common group appeared right at the module root (https://doc.riot-os.org).
This PR adds (or corrects) a dedicated doxygen group for every board group in
boards/common
and also fixes grouping for STM nucleos. I know there is (a lot) more to do but another step is taken here to enhance the documentation.