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

Set default --no-status to CATKIN_TOOLS_BUILD_OPTIONS #297

Merged
merged 1 commit into from
Aug 11, 2016

Conversation

wkentaro
Copy link
Member

For #296 (comment)

Could u please give me feedbacks? @furushchev @k-okada

This commit fixes belows:

  • Typo "ROS_DISTRO" should be "$ROS_DISTRO", but checking catkin-tools
    version is better.
  • Replace --limit-status 0.002 with --no-status the status limit
    should be specified in .travis.yml like
    export CATKIN_TOOLS_BUILD_OPTIONS="-iv --summarize --limit-status 0.001".

@wkentaro wkentaro force-pushed the fix-catkin-build-opt branch from 6f5d717 to 74f69c7 Compare August 10, 2016 17:50
@furushchev
Copy link
Member

furushchev commented Aug 11, 2016

@wkentaro

Replace --limit-status 0.002 with --no-status the status limit should be specified in .travis.yml like export CATKIN_TOOLS_BUILD_OPTIONS="-iv --summarize --limit-status 0.001"

Why do you think so? Jenkins is also run in travis, and no output longer than 10min lead travis to be dead anyway.

Typo "ROS_DISTRO" should be "$ROS_DISTRO", but checking catkin-tools version is better.

Nice catch I created another PR to fix only this problem.

@wkentaro
Copy link
Member Author

Why do you think so? Jenkins is also run in travis, and no output longer than 10min lead travis to be dead anyway.

Because the build on Jenkins does not need --limit-status 0.002 because it requests the log once after the build.
That's why we did not need --limit-status 0.002 option as the default to build jsk_recognition package.
https://github.com/jsk-ros-pkg/jsk_travis/blob/master/travis_jenkins.py#L413

@k-okada
Copy link
Member

k-okada commented Aug 11, 2016

sorry need rebase.?

This commit fixes belows:

- Typo "ROS_DISTRO" should be "$ROS_DISTRO", but checking catkin-tools
  version is better.
- Replace `--limit-status 0.002` with `--no-status` the status limit
  should be specified in .travis.yml like
  `export CATKIN_TOOLS_BUILD_OPTIONS="-iv --summarize --limit-status 0.001"`.
@wkentaro
Copy link
Member Author

Rebased.

@k-okada k-okada merged commit 8f99f47 into jsk-ros-pkg:master Aug 11, 2016
@wkentaro wkentaro deleted the fix-catkin-build-opt branch August 11, 2016 14:12
wkentaro added a commit to wkentaro/jsk_travis that referenced this pull request Aug 11, 2016
furushchev pushed a commit to furushchev/jsk_travis that referenced this pull request Aug 17, 2016
wkentaro added a commit to wkentaro/jsk_recognition that referenced this pull request Sep 16, 2016
wkentaro added a commit to wkentaro/jsk_recognition that referenced this pull request Sep 21, 2016
wkentaro added a commit to wkentaro/jsk_recognition that referenced this pull request Oct 10, 2016
mizmizo pushed a commit to mizmizo/jsk_recognition that referenced this pull request Apr 26, 2017
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