Skip to content

remove catkin_lint suppressions#1343

Merged
rhaschke merged 1 commit intomoveit:melodic-develfrom
ubi-agni:catkin-lint
Feb 18, 2019
Merged

remove catkin_lint suppressions#1343
rhaschke merged 1 commit intomoveit:melodic-develfrom
ubi-agni:catkin-lint

Conversation

@rhaschke
Copy link
Copy Markdown
Contributor

@rhaschke rhaschke commented Feb 12, 2019

fkie/catkin_lint#62 has been resolved and catkin_lint 1.5.6 has been released.
Thus, we can remove some catkin_lint suppressions again.


#suppress spurious error: https://github.com/fkie/catkin_lint/issues/62
#catkin_lint: ignore_once external_directory missing_directory (${VERSION_FILE_PATH})
#catkin_lint: ignore_once external_directory (${VERSION_FILE_PATH})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like you removed the comment explaining and linking to the github issue, but kept the ignore_once - is that what you intended to do?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(or was that issue only for the missing_directory)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the issue was related to missing_directiory. The external_directoring warning is legitimate but should be suppressed here.

Comment thread moveit_core/CMakeLists.txt Outdated
# replace LIBFCL_LIBRARIES with full path to the library
find_library(LIBFCL_LIBRARIES_FULL ${LIBFCL_LIBRARIES} ${LIBFCL_LIBRARY_DIRS})
set(LIBFCL_LIBRARIES "${LIBFCL_LIBRARIES_FULL}")
include(FindPkgConfig)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This change is a bit different than the rest of this PR - the rest is removing suppresions for caktin_lint; this appears to be updating the way we use pkg_config from cmake. Seems innocuous though, so I'm happy to have it in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is related to this PR, because it successfully suppresses another catkin_lint warning as suggested in fkie/catkin_lint#62 (comment).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You should keep the original find_package(PkgConfig REQUIRED) though. It's the recommended way of using CMake find modules.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the hint. I thought include(FindPkgConfig) and find_package(PkgConfig REQUIRED) are synonymous.

... after fkie/catkin_lint#62 has been resolved
and catkin_lint 1.5.6 has been released.
@rhaschke rhaschke merged commit dbde093 into moveit:melodic-devel Feb 18, 2019
@rhaschke rhaschke deleted the catkin-lint branch February 20, 2019 07:41
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
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