Skip to content
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.

Sign up for GitHub

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

Jump to bottom

net/cord doc: Usability fixes #16812

Merged
merged 6 commits into from
Apr 26, 2022
Merged

net/cord doc: Usability fixes #16812

merged 6 commits into from
Apr 26, 2022

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Sep 5, 2021

Contribution description

The CoRD documentation had a few shortcomings, some of which doxygen already warned about, some from general usability, some because the text I added in #16113 was a bit too concise.

Testing procedure

  • Documentation builds without errors (even though some exclude patterns from dist/tools/doccheck: add exclude file for warnings and use it  #16779 were removed)
  • All CoRD configuration macros are shown and searchable (for example, CONFIG_CORD_EP used to be shown but could not be found through the search; CONFIG_CORD_EXTRAARGS was not shown before at all).

@chrysn chrysn added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: doc Area: Documentation Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Sep 5, 2021
@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System Area: tools Area: Supplementary tools and removed Area: doc Area: Documentation labels Sep 5, 2021
@chrysn chrysn added Area: doc Area: Documentation State: waiting for CI update State: The PR requires an Update to CI to be performed first and removed Area: network Area: Networking Area: tools Area: Supplementary tools Area: sys Area: System labels Sep 5, 2021
@chrysn
Copy link
Member Author

chrysn commented Sep 5, 2021 •
edited
Loading

As this removes doxygen exclusions from 16779, to avoid holding up the upcoming CI changes of RIOT-OS/riotdocker#104 with an accidental regression, this should wait for the new riotdocker images to run the checks against.

[edit: I did run the static-tests locally, but there doxygen shows additional warnings from unrelated modules; maybe due to an even newer Doxygen version?]

@github-actions github-actions bot added Area: network Area: Networking Area: sys Area: System Area: tools Area: Supplementary tools and removed Area: doc Area: Documentation labels Sep 5, 2021
@chrysn chrysn force-pushed the cord-doc-discoverability branch from b821b0f to 8559d66 Compare September 5, 2021 09:18
@chrysn chrysn added Area: doc Area: Documentation and removed Area: tools Area: Supplementary tools Area: sys Area: System labels Sep 5, 2021
@fjmolinas
Copy link
Contributor

It doesn't seem to need the CI update anymore no?

@github-actions github-actions bot added Area: sys Area: System Area: tools Area: Supplementary tools and removed Area: doc Area: Documentation labels Jan 27, 2022
@chrysn chrysn removed Area: tools Area: Supplementary tools Area: sys Area: System labels Jan 27, 2022
@chrysn chrysn added Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Jan 27, 2022
@chrysn
Copy link
Member Author

chrysn commented Jan 27, 2022

Indeed; flag removed and miri's suggestions pulled in.

As there were no more review items other than the one that is simple to follow, but warnings about merge conflicts, I'm taking the liberty to rebase.

chrysn and others added 5 commits January 27, 2022 09:47
@chrysn
net/cord doc: Rename configuration section
da03ee8 
The title is long enough that it is easily misread as "Client
Configuration" and skipped when looking for common configuration.

(Plus none of the other submodules of net_cord spell out the full "Core
RD Endpoint and Lookup Client" module name again,a dn all say "CoRE RD
Something".)
@chrysn
net/cord doc: Fix unbalanced grouping
ca25ab9
@chrysn @miri64
net/cord doc: Unify configuration documentation ...
7253817 
... anchored both under "compile time configuration" and "Networking /
CoRE RD"

This makes the related configuration items visible on the same page.

Co-authored-by: Martine Lenders <mail@martine-lenders.eu>
@chrysn
net/cord doc: Fix Endpoint ID grouping
72c21f6 
The braces (which appear to do nothing) were keeping Doxygen from
grouping the three "Endpoint ID defition" parameters (apparently
intended to be grouped from the braces in the file) from being shown
together.

Moreover, they kept CONFIG_CORD_EXTRAARGS from showing.
@chrysn
net/cord doc: Expand CONFIG_CORD_EXTRAARGS documentation
418dafd
@chrysn chrysn force-pushed the cord-doc-discoverability branch from b6ae295 to a67336f Compare January 27, 2022 08:54
@github-actions github-actions bot added Area: sys Area: System Area: tools Area: Supplementary tools and removed Area: doc Area: Documentation labels Jan 27, 2022
@chrysn chrysn force-pushed the cord-doc-discoverability branch from a67336f to 2caa1bd Compare January 27, 2022 08:56
@chrysn chrysn added Area: doc Area: Documentation and removed Area: tools Area: Supplementary tools Area: sys Area: System labels Jan 27, 2022
miri64
miri64 approved these changes Apr 26, 2022
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK, but could you re-trigger the static tests?

@miri64 miri64 added the CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs label Apr 26, 2022
@chrysn chrysn added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 26, 2022
@chrysn chrysn force-pushed the cord-doc-discoverability branch from 2caa1bd to 92e877b Compare April 26, 2022 16:52
@github-actions github-actions bot added Area: sys Area: System Area: tools Area: Supplementary tools and removed Area: doc Area: Documentation labels Apr 26, 2022
@chrysn chrysn enabled auto-merge April 26, 2022 16:53
@miri64
Copy link
Member

miri64 commented Apr 26, 2022

Thx!

@chrysn chrysn merged commit 0a72fc9 into RIOT-OS:master Apr 26, 2022
@chrysn chrysn deleted the cord-doc-discoverability branch April 26, 2022 19:35
@chrysn chrysn added this to the Release 2022.07 milestone Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@benpicco benpicco Awaiting requested review from benpicco

Assignees
No one assigned
Labels
Area: network Area: Networking Area: sys Area: System Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2022.07
Development

Successfully merging this pull request may close these issues.
3 participants
@chrysn @fjmolinas @miri64