ros2_control
ros2_control copied to clipboard
ros-controls
pep257 differences between the ros-tooling and ros-industrial CI
By @Karsten1987
I do believe there's a fine difference in the pep257 between the ros-tooling and ros-industrial CI. The source build (ros-tooling) seems to use a version which expects a semicolon ; after a doc section, whereas the binary jobs (ros-industrial) don't. Not sure what to do here.
https://github.com/ros-controls/ros2_control/pull/349
https://github.com/ros-controls/ros2_control/pull/349/commits/0df21e18a8f8c7e5941034344a5222c8e29cf19c
I think we should simply remove the non-tests from https://github.com/ros-controls/ros2_control/tree/master/ros2controlcli/test and let the linter workflows do their job. Those individual scripts are prune to be flaky depending on ones installed python package versions which is I believe what we are seeing with the industrial CI.
why not to move this into pre-commits. See here: https://www.youtube.com/watch?v=tOlnfrctpSY
what does this mean to remove the non-tests? There aren't any inbuilt linters for python. THese files are unfortunately copied in every package which has python code, e.g: https://github.com/ros2/demos/tree/master/demo_nodes_py/test
On Fri, Apr 2, 2021 at 12:43 AM Denis Štogl @.***> wrote:
why not to move this into pre-commits. See here: https://www.youtube.com/watch?v=tOlnfrctpSY
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ros-controls/ros2_control/issues/356#issuecomment-812393820, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJLGYI5XIFDYY3FEKHPJWDTGVYTJANCNFSM42BO4W3A .
Yes and that is a problem. We are carrying dead weight. Don't want to follow a bad example. Precommit for instance is able to reliably reproduce linting work both locally editing (some) and CI validating without the flakiness of dependig and on which pytest or other tools are installed in the environment.
sure, I am happy to have a better solution for this - as long as it's kind of transparently incorporated so that I can be able to test locally and have CI do the exact same thing as well.