ros-naoqi / naoqi_driver

c++ bridge based on libqi
Apache License 2.0
51 stars 93 forks source link

Adapt the README to the new CI #130

Closed mbusy closed 4 years ago

mbusy commented 4 years ago

Modify the CI section of the README to match the new CI. Currently only Ubuntu distros are targeted for the tests (guessed from ROS_DISTRO), the CI could be extended with Debian distros, etc. (see here and here)

mikaelarguedas commented 4 years ago

Oh yeah I forgot to update the column name I just made sure the indigo and kinetic ones were valid >_< thanks for the follow-up Actually I think it makes sense to add melodic stretch ci right away

TBH I'm not sure having multiple columns make sense. I'd suggest having just a table with the first column reflecting the ROS (+debian ID if applicable) and status, ala

+-----------------+---------------------+
|   ROS Release   |       status        |
+=================+=====================+
| Melodic-stretch |  |melodic-stretch|  |
+-----------------+---------------------+
| Melodic         |     |melodic|       |
+-----------------+---------------------+
| Kinetic         |     |kinetic|       |
+-----------------+---------------------+
| Indigo          |     |indigo|        |
+-----------------+---------------------+

Reason being that ROS distro always mainly target the matching ubuntu LTS and some other distros on the side (e.g debian LTS or non-LTS ubuntu) In this case we would be matching the docker tags:

mbusy commented 4 years ago

It made sense when lunar was still in the CI (xenial/stretch), but it's not anymore. It would also make sense if Noetic was targeting an already mentioned OS, although I'm guessing that Focal will be targeted... In that case, we can use a simpler table

  • ros:melodic points to ros:melodic-ros-base-bionic
  • If you want another distro e.g. stretch you need to specify explicitly: ros:melodic-ros-base-stretch

So you're saying that instead of specifying the OS_NAME and OS_CODE_NAME variable, ROS_DISTRO can be directly edited (for instance melodic -> melodic-ros-base-stretch) ?

mbusy commented 4 years ago

Tried some changes locally, the last commit should add melodic stretch to the CI

mikaelarguedas commented 4 years ago

although I'm guessing that Focal will be targeted... In that case, we can use a simpler table

Yeah Noetic will target Ubuntu Focal and Debian Buster https://github.com/ros-infrastructure/rep/pull/202/files#diff-41ae062315c36c45afaa5c192b2ca7c5R372

So you're saying that instead of specifying the OS_NAME and OS_CODE_NAME variable, ROS_DISTRO can be directly edited (for instance melodic -> melodic-ros-base-stretch) ?

Sorry that was unclear, I just meant that in the case of the default OS (e.g Melodic on bionic) we don't need to be explicit in the table.