Conversation
|
I'm marking this PR as draft because it is blocked by some PRs in ament/ament_lint, namely ament/ament_lint#327, ament/ament_lint#328, ament/ament_lint#329, ament/ament_lint#330, and a forthcoming PR to address file exclusion behavior in The Rpr job is failing because said PRs haven't landed. |
clalancette
left a comment
There was a problem hiding this comment.
The changes here look generally fine to me, though in order to move forward here we need to:
- Rebase this on the latest
- Get the ament PRs in
- Get green CI here
The copyright header in test/test_time.cpp was failing `ament_copyright` checks otherwise. Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
`ament_cpplint` reported a few formatting issues, all to do with lines longer than 100 characters. These issues have been addressed with `ament_uncrustify --reformat`. Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
Each of the common linters were individually `find_package`-d and invoked in the CMakeLists.txt file. This is because there is no simple way to pass file exclusion information through `ament_lint_auto`. File exclusion is necessary because we do not want to run linters on the include/tf2/LinearMath/ headers, since these are external and periodically synced with upstream. See #258 (comment) for more information. Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
Will do. By the way, @clalancette as I was rebasing on the ros2 branch, I noticed that #466 and #467 were rebased onto that branch rather than squash-and-merged. |
32a7ae1 to
e7e279c
Compare
Yeah, that was on purpose. The changes were different enough in each commit that I didn't think squashing was appropriate. |
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
|
Marking this PR as ready-for-review following release 0.11.3 of ament_lint, which introduced most of the lint infrastructure needed for this PR. CI:
|
Signed-off-by: Abrar Rahman Protyasha <aprotyas@u.rochester.edu>
|
Green CI + approval, so I'll merge this. Thanks for the review! |
As discussed in #467, this PR:
ament_lint_commonintf2.Note that individual linters had to be invoked rather than use
ament_lint_autobecause the LinearMath headers should not have linters run on them - see #258 (comment) for more information - and becauseament_lint_autodoes not provide a general way of providing file exclusion lists.Signed-off-by: Abrar Rahman Protyasha aprotyas@u.rochester.edu