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.

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

Add documentation of log levels #93

Merged
merged 2 commits into from
May 15, 2020
Merged

Add documentation of log levels #93

merged 2 commits into from
May 15, 2020

Conversation

tfoote
Copy link
Contributor

@tfoote tfoote commented Apr 2, 2020

## https://docs.python.org/3/library/logging.html#logging-levels
## And are implemented in rcutils as well
## https://github.com/ros2/rcutils/blob/35f29850064e0c33a4063cbc947ebbfeada11dba/include/rcutils/logging.h#L164-L172
## This leaves space for other logging levels to be inserted in the middle in the future.
Copy link
Member

Choose a reason for hiding this comment

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

Not only in the future but custom user defined levels.

rcl_interfaces/msg/Log.msg Outdated Show resolved Hide resolved
rcl_interfaces/msg/Log.msg Outdated Show resolved Hide resolved
@hidmic
Copy link
Contributor

hidmic commented Apr 16, 2020

@tfoote friendly ping.

@tfoote tfoote requested a review from dirk-thomas May 15, 2020 08:49
## This leaves space for other standard logging levels to be inserted in the middle in the future,
## as well as custom user defined levels.
## Since there are several other logging enumeration standard for different implementations,
## other logging implementations may need to provide level mappings to match their internal implementations.
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity: why two #? Are these comments still being extracted properly and used in the generated code without any leading #?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was just copying the existing style.

@tfoote tfoote merged commit 018fe20 into master May 15, 2020
@delete-merged-branch delete-merged-branch bot deleted the tfoote-patch-1 branch May 15, 2020 23:05
@dirk-thomas
Copy link
Member

Please double check the docblocks in the generated code. That was also part of the review if I recall correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants