Add ci

[M1chaelM]

This PR will add CI to our vorc repo. It still needs some tweaking but I am creating the PR so I can see whether the github actions are working.

[M1chaelM]

This seems to be working, with a few caveats:

• We have no cplusplus code to check, so the static checks are currently not looking at anything.
• I've switched over to trying to use colcon to build and test. This seems to be working but there are a lot of warnings that I'm not familiar with.

@mabelzhang would you be able to look over the output of the checks in the Build and Run section and verify that these are sane? (Also, is the colon test command the appropriate replacement for catkin_make run_tests?).

[mabelzhang]

Thanks! I have just the thing - will be adding C++ code in my next PR. Yes to colcon test. Will look at this PR after I have some C++ code ready.

[M1chaelM]

Thanks! I have just the thing - will be adding C++ code in my next PR. Yes to colcon test. Will look at this PR after I have some C++ code ready.

Fantastic. The code_check.sh will need to be modified so it looks for it. If it's going in vorc_gazebo/src and vorc_gazebo/include I can add these now. Otherwise I just need to not forget. :)

[mabelzhang]

The new source code from #22 is in vorc_gazebo/include/vorc_gazebo/gymkhana_scoring_plugin.hh and vorc_gazebo/src/gymkhana_scoring_plugin.cc . Thanks!

[M1chaelM]

Thanks @mabelzhang . This seems to be working as long as the build output doesn't look weird to you.

[mabelzhang]

Looks nice. Thanks for setting this up!

I see what you mean about the warnings.

tl;dr: These warnings happen with catkin locally for me as well. Most of them come from FindBoost.cmake, which we cannot fix on our end, so we'll have to set the policy flags downstream. The solution for properly getting rid of them is to set policies to NEW or OLD in CMakeLists.txt (from https://github.com/BeamMW/beam/commit/cffe5a6d2cb2e32d859568818476e61d7ff30528 ):

if (POLICY CMP0054)
  cmake_policy(SET CMP0054 OLD)
endif()

I've done so for VRX packages in https://github.com/osrf/vrx/pull/226 . I took the liberty to push such changes for VORC packages in 3686894 to see quick CI turnaround.

Details:

These show that the new policies were introduced in CMake 3.1+:

$ cmake --help-policy CMP0046
$ cmake --help-policy CMP0054
$ cmake --help-policy CMP0071

46 is about whether to silently fail when dependencies in add_dependencies() are not found. (3.10.2) 54 is related to whether variables within quotes in if() statements should be seen as variables. (3.1) 71 is specific to AUTOMOC and AUTOUIC, which vrx_gazebo explicitly sets to ON for some of the Qt files that it needs to build. (3.10)

In summary, these are good policies (in my opinion) and impose stricter checks.

Since Ubuntu 18.04 (official platform for Melodic) apt ships CMake 3.10.2, I suspected that catkin would give the same warnings. So I tried

$ rm -rf build install log
$ catkin_make

and there were CMP0046 and CMP0071 warnings on vrx_gazebo, and CMP0054 on some other Gazebo dependencies, etc.

So it's just a matter of updating both VRX and VORC repositories to follow the newer CMake standards. Even though they're just warnings, I think it's a good idea to get rid of all warnings so that the output is short and clean for easier inspections in the future.

If they were from our own code in CMakeLists.txt, then it is best to simply update to follow the new standards. However, since they are mostly upstream, we'll have to set the flags to OLD until they're fixed upstream.

Perhaps one exception is CMP0046 in vrx_gazebo, which if I set to NEW, it errors about *_generate_services_cpp not existing. That could be updated to simply put msgs and srvs in a vrx_gazebo_msgs package, and have vrx_gazebo depend on it, instead of going the _generate_services_cpp route. Then it will follow the new policy.

[mabelzhang]

Comparing GitHub workflow on the previous 53d1db2 https://github.com/osrf/vorc/pull/21/checks?sha=53d1db239073a2e99c256c6537a9eacb24d63ae7 and the latest 3686894 https://github.com/osrf/vorc/pull/21/checks?check_run_id=1329798780 , looks like the warnings on the VORC packages cora_descirption and vorc_gazebo are gone.

The only ones left are the VRX ones which are pending merge over there.

[M1chaelM]

Thanks @mabelzhang ! I really appreciate this breakdown. I will go ahead and merge this now.