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

Travis: enable warnings and catkin_lint checker #1332

Merged
merged 6 commits into from
Feb 4, 2019

Conversation

rhaschke
Copy link
Contributor

@rhaschke rhaschke commented Feb 1, 2019

This PR enables more checkers in Travis using the new features of moveit/moveit_ci#45.

  • increase Travis timeout (90min - 5min margin)
  • update Travis distro to Xenial as suggested by Travis folks
  • add catkin_lint (currently failing)
  • enable warnings checker (failing on clang)

Fixes for clang warnings will immediately follow.

Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Approved after my small suggestions

.travis.yml Outdated
@@ -1,6 +1,6 @@
# This config file for Travis CI utilizes https://github.com/ros-planning/moveit_ci/ package.
sudo: required
dist: trusty
dist: xenial
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
dist: xenial
# This is the distro ran by Travis, which may or may not be what MoveIt! is tested against within Docker
dist: xenial

.travis.yml Outdated
@@ -13,12 +13,15 @@ notifications:
- 130s@2000.jukuin.keio.ac.jp
env:
global:
- MOVEIT_CI_TRAVIS_TIMEOUT=85
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- MOVEIT_CI_TRAVIS_TIMEOUT=85
- MOVEIT_CI_TRAVIS_TIMEOUT=85 # Travis allows us 90 min but we add a safety margin to avoid timeouts

.travis.yml Outdated
@@ -29,6 +32,9 @@ matrix: # Add a separate config to the matrix, using clang as compiler
env: ROS_REPO=ros-shadow-fixed
BEFORE_DOCKER_SCRIPT="source moveit_kinematics/test/test_ikfast_plugins.sh"

allow_failures:
- env: TEST=catkin_lint
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- env: TEST=catkin_lint
- env: TEST=catkin_lint # TODO: fix all catkin_lint errors in the future

@@ -1,5 +1,9 @@
set(MOVEIT_LIB_NAME moveit_robot_state)

if(CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
Copy link
Member

Choose a reason for hiding this comment

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

could you add a comment about why this is necessary please?

+ increase Travis timeout (90min - 5min margin)
+ update Travis distro to Xenial as suggested by Travis folks
+ add catkin_lint (currently failing)
- add missing parentheses
- correct comparison operator
@rhaschke rhaschke merged commit 9dc737c into moveit:melodic-devel Feb 4, 2019
@rhaschke rhaschke deleted the enable-warnings branch February 4, 2019 08:01
sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024
Co-authored-by: AndyZe <zelenak@picknik.ai>
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.

None yet

2 participants