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

Added travis integration #75

Merged
merged 1 commit into from
Apr 2, 2019
Merged

Added travis integration #75

merged 1 commit into from
Apr 2, 2019

Conversation

bsaund
Copy link
Contributor

@bsaund bsaund commented Apr 1, 2019

Travis web page: https://travis-ci.com/UM-ARM-Lab/arc_utilities
(I'm curious if other UM-ARM-Lab github users have access automatically)

I'll add the travis link+image to the readme after this is merged in. Note the build on master is currently "failing" because there is no travis integration on master.
Build Status

@bsaund bsaund requested review from a-price and cragoo April 1, 2019 15:12
@cragoo
Copy link

cragoo commented Apr 1, 2019

I can see the travis page

Copy link

@cragoo cragoo left a comment

Choose a reason for hiding this comment

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

Nice, looks good to me. Is the setup script needed if you just ending executing catkin_make_isolated in test.sh anyways?

@bsaund
Copy link
Contributor Author

bsaund commented Apr 1, 2019

setup.sh is not necessary. In other projects (see: mps_shape_completion) I use the setup script to pull in dependency repos.
I am including setup.sh

  1. for consistency across projects
  2. To make it easier for some later person to find a place to do additional setup, in case that is every needed

@cragoo
Copy link

cragoo commented Apr 1, 2019

Fair enough. Often I think these scripts are rarely looked at, so reducing the footprint of build files to a minimum makes it easier to sleuth build issues later on. Here that could be done by leaving the setup file empty except for a comment indicating that more code can be added.

In this case the files are pretty small so it doesn't matter much, but I'd thought I'd offer an alternative opinion.

@bsaund bsaund merged commit 3fad039 into master Apr 2, 2019
@bsaund bsaund mentioned this pull request Apr 4, 2019
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.

2 participants