Skip to content

net/gcoap: add config macros to config doc group#10676

Merged
miri64 merged 3 commits intoRIOT-OS:masterfrom
kb2ma:gcoap/doc_config_group
Jan 3, 2019
Merged

net/gcoap: add config macros to config doc group#10676
miri64 merged 3 commits intoRIOT-OS:masterfrom
kb2ma:gcoap/doc_config_group

Conversation

@kb2ma
Copy link
Copy Markdown
Member

@kb2ma kb2ma commented Dec 29, 2018

Contribution description

Adds a documentation group for the user configurable macros in gcoap, per #10566.

Also retitles the gcoap module in the source documentation from "CoAP" to "Gcoap" for clarity. We also have a Nanocoap module as well as a CoAP defines module. I plan to add a similar documentation group for the configurable macros in these other modules as well, which really triggered me to retitle here to avoid further confusion.

Testing procedure

Compile the documentation. You should see:

  • a new configuration group titled, "Gcoap compile configurations"
  • the main gcoap module move from "CoAP" to "Gcoap"
  • a new module within Gcoap, also titled "Gcoap compile configurations"

Issues/PRs references

Partially implements #10566

@kb2ma kb2ma added Area: doc Area: Documentation Area: CoAP Area: Constrained Application Protocol implementations labels Dec 29, 2018
@kb2ma kb2ma requested review from jia200x and miri64 December 29, 2018 16:08
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Dec 30, 2018

Pushed fixups and new commit for retitle of documentation group. Looking for a response on comment about ordering of configurable macros.

Avoids confusion with generic CoAP and capitalization retains order
in documentation.
@kb2ma kb2ma force-pushed the gcoap/doc_config_group branch from 233fd61 to beae37c Compare December 30, 2018 15:46
@kb2ma kb2ma added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 30, 2018
@kb2ma
Copy link
Copy Markdown
Member Author

kb2ma commented Dec 30, 2018

Squashed and running CI build.

Copy link
Copy Markdown
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.

Still found something, otherwise I'm fine now with the PR as is.

/**
* @brief Length in bytes for a token; use 2 if not defined
* @ingroup net_gcoap_conf
* @brief Length in bytes for a token
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this supposed to be in any relation to GCOAP_TOKENLEN_MAX? If yes please document.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch. Documented in 03d4230.

@jia200x jia200x added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 3, 2019
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Jan 3, 2019

documentation is generated accordingly and all GCOAP macros from #10566 are included here

@jia200x jia200x added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jan 3, 2019
Copy link
Copy Markdown
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

ACK. @miri64 are you ok with this PR?

Copy link
Copy Markdown
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.

Yep.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Jan 3, 2019

Let's go! :-)

@miri64 miri64 merged commit 019c1ab into RIOT-OS:master Jan 3, 2019
@aabadie aabadie added this to the Release 2019.01 milestone Jan 4, 2019
@kb2ma kb2ma deleted the gcoap/doc_config_group branch February 4, 2019 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: CoAP Area: Constrained Application Protocol implementations Area: doc Area: Documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants