Update DoCIF and use ros:indigo baseimage #202
Conversation
|
I would also try rebuilding master to see if it was an infra issue at all. Could you turn on 'build forked PR's' in circleci? I reccomend not turning on buiild secrets though.
|
|
Done! Thanks for the help Jay! I did try rebuilding master. I mentioned it didn't work here: 34d006a |
|
Actually I'm gonna try rebuilding again |
jgkamat
force-pushed
the
master
branch
3 times, most recently
from
e3c0d54
to
e0fa934
Compare
jgkamat
force-pushed
the
master
branch
2 times, most recently
from
d0daf97
to
f9e2c70
Compare
|
Hey, it looks like the CI magically started working again (so it was probably a bug in circleci or a dockerimage that was fixed). This branch does have a few improvements though (such as pulling from ros:indigo instead of installing ros for every build), so you can use it if you want. Let me know if you want it cleaned up. :P |
|
Yeah I saw it was fixed. Like 2 days after you last pushed to this PR I rebuilt master and it passed :) I am fine with this PR, let me just make a comment or two and if you have some time to clean it up I'd appresh <3 |
circle.yml
Outdated
| @@ -14,9 +15,6 @@ dependencies: | |||
| - ./DoCIF/commands/buildbaseimage.sh | |||
| # Actually the test step | |||
| - ./DoCIF/commands/runtests.sh | |||
| cache_directories: | |||
| # This shouldn't actually cache anything because config.docif says not to | |||
| - ~/.ccache # See $CACHE_DIRECTORIES in config.docif | |||
There was a problem hiding this comment.
So we have plans to cache things eventually (#156). I understand this is not needed now, but I don't really see a reason to remove it :x
|
I'm pretty confused why preventing rosdep init would cause it to timeout (afaik, the other way is failing, and not doing anything). I'll take a closer look at that later. |
jgkamat
requested review from
chsahit,
coletaylor788 and
joshuamorton
as code owners
install
Outdated
|
|
||
| # --------- | ||
| # Everything above this point has been installing various python dependencies | ||
| # for things we do. Stuff below this is the ros build and install. | ||
|
|
||
| sudo rosdep init # This can only be run once. Possible enhancement, check this | ||
| sudo rosdep init || true # This can only be run once. Possible enhancement, check this |
There was a problem hiding this comment.
Could you upgrade this to a TODO?
|
The tests actually did run (see the circleci build logs). Forked PR's don't get secrets (doing so would be a security risk), so that's why the individual status tokens aren't updating. I'll fix up the PR and merge master into it. This does not help with #192 in any way though (as far as I can tell). I'll also trash that 'bump' commit. |
|
Ah yeah, then LGTM. |
Hopefully this might affect #201
We'll see if the build works. There are a few hacky things in here, so don't merge this yet (still a WIP).