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

Document new deprecation approach, and TODO format #379

Merged
merged 2 commits into from
Nov 21, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 26 additions & 4 deletions documentation/contributing/code/index.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,35 @@ In addition MoveIt has some extra style preferences:
- Use the C++ standard library (``std::``) whenever possible
- Avoid C-style functions such as ``FLT_EPSILON`` - instead use ``std::numeric_limits<double>::epsilon()``
- Boost is an encouraged library when functionality is not available in the standard library
- Prefer full variable names over short acryonms - e.g. ``robot_state_`` over ``rs_``
- Deprecate functions using MOVEIT_DEPRECATED in [deprecation.h](https://github.com/ros-planning/moveit/blob/master/moveit_core/macros/include/moveit/macros/deprecation.h)
- Use "pragma once" in headers instead of include guards.

## Inline Documentation

- We use Doxygen-style comments
- To document future work, use the format ``TODO(username): description``
- Add extensive comments to explain complex sections of code
- Prefer full, descriptive variable names over short acryonms - e.g. ``robot_state_`` over ``rs_``
- Avoid ``auto``, if the variable type doesn’t become clear immediately from the context (e.g. from ``make_shared<...>``)

## Deprecation

- Deprecate functions using C++14 [``[[deprecated]]``](https://en.cppreference.com/w/cpp/language/attributes/deprecated) attribute
- Add a useful message describing how to handle the situation:

[[deprecated("use bar instead")]] void foo() {}

Which will result in:

warning: 'foo' is deprecated: use bar instead [-Wdeprecated-declarations] foo(); ^ note: 'foo' has been explicitly marked deprecated here void foo() {} ^

- Add an associated TODO describing when to remove the feature (date and/or ROS version)

## Exceptions

- Catch known exceptions and log them in detail. Avoid using ``catch (...)`` as it hides every information of a possible fault. We want to know if something goes wrong.
- We don't catch exceptions that don't derive from ``std::exception`` in MoveIt. It is the responsibility of the plugin provider to handle non-``std::exception``-derived exceptions locally.
- Use "pragma once" in headers instead of include guards.

## ROS
## Logging

- The ROS logging functionality is utilized and namespaced. i.e. ``ROS_INFO_NAMED(LOGNAME, "Starting listener...``.
- This makes it easier to understand where output is coming from on the command line and allows for more fine-grained filtering of terminal output noise.
Expand Down