search

ros-drivers / transport_drivers

A set of ROS2 drivers for transport-layer protocols.
Apache License 2.0
92 stars 56 forks source link

update CI to standard ros-tooling github actions #25

Closed flynneva closed 3 years ago

flynneva commented 3 years ago

the current CI strategy for this repo looks pretty custom (straight up bash commands in a list).

would we be able to switch over to the more standard ros-tooling/setup-ros and ros-tooling/actions-ros-ci.

ive went ahead and made these changes on my fork of this repo if you'd like to check it out.

flynneva commented 3 years ago

more than happy to make a PR with the changes once given the "green light"

JWhitleyWork commented 3 years ago

I've had a lot of problems in the past with ros-tooling/setup-ros. Specifically, they do a lot of tests and commands that aren't necessary in most cases and when something changes in a ROS package that they use, it breaks their jobs and the only way to fix it is to wait for them to find the bug, release a new version, then for the maintainer of the repos that use it to update their version. While the CI job that is used now may look custom, it's very standardized. The only part that would make it more portable to other repos is to replace the repo name in the script with $GITHUB_REPOSITORY. This adds an extra directory but everything would still work fine.

flynneva commented 3 years ago

that may have been true about setup-ros a few months ago....it definitely isnt today. go ahead and look at what it runs. it is (almost exactly) the commands you are running in your setup.

JWhitleyWork commented 3 years ago

I'm willing to give it a shot again. Go ahead with the PR.

flynneva commented 3 years ago

closed with #34