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

Clean Codebase #102

Closed
1 of 6 tasks
henningkayser opened this issue Oct 18, 2019 · 2 comments
Closed
1 of 6 tasks

Clean Codebase #102

henningkayser opened this issue Oct 18, 2019 · 2 comments
Assignees

Comments

@henningkayser
Copy link
Member

henningkayser commented Oct 18, 2019

General (non-technical) cleanup steps and linting tools for better consistency in the code and maintainability.

TODOs

  • ament_lint, clang-tidy-fix (Enable clang-tidy-fix and ament_lint_cmake #210)
  • Unify logging, apply logger name conventions across codebase
  • Use RCL_ROS_TIME where applicable
  • Remove redundant meta packages
  • Update documentation, cleanup ROS 1 references (i.e. NodeHandle, parameter server... )
  • Remove commented and deprecated code, outdated TODO's

Old description:

In order enforce clean coding standards using clang-format, clang-tidy (others?) we should enable ament-lint for all packages (https://github.com/ament/ament_lint).

Related/duplicate with #98. (https://ubuntu.com/blog/how-to-add-a-linter-to-ros-2)

@henningkayser henningkayser self-assigned this Oct 18, 2019
JafarAbdi pushed a commit to JafarAbdi/moveit2 that referenced this issue Oct 28, 2019
…it_struct_forward

Removed coma MOVEIT_STRUCT_FORWARD to avoid warnings
@henningkayser

This comment has been minimized.

@henningkayser
Copy link
Member Author

henningkayser commented Jun 12, 2020

There are also a bunch of other linters that we can check for in CI or run with ament. I'm thinking of enabling ament_cpp_common at some point, but we probably want to maintain our own ruleset for this. The next ones to enable are imo:

  • ament_cppcheck - only shows 11 errors in MoveIt for me
  • ament_cpplint - shows 3618 errors with --linelength set to 120, but thiese include a lot of formatting issues (i.e. else should be in same line as }) and overall redundancy, so maybe we can get this enabled with our own rules while keeping the diff to MoveIt1 minimal

MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this issue Aug 15, 2022
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

No branches or pull requests

1 participant