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

pep257 differences between the ros-tooling and ros-industrial CI #356

Closed
bmagyar opened this issue Mar 30, 2021 · 5 comments · Fixed by #1626
Closed

pep257 differences between the ros-tooling and ros-industrial CI #356

bmagyar opened this issue Mar 30, 2021 · 5 comments · Fixed by #1626

Comments

@bmagyar
Copy link
Member

bmagyar commented Mar 30, 2021

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.

#349

0df21e1

@bmagyar
Copy link
Member Author

bmagyar commented Mar 30, 2021

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.

@destogl
Copy link
Member

destogl commented Apr 2, 2021

why not to move this into pre-commits. See here: https://www.youtube.com/watch?v=tOlnfrctpSY

@Karsten1987
Copy link
Contributor

Karsten1987 commented Apr 2, 2021 via email

@bmagyar
Copy link
Member Author

bmagyar commented Apr 4, 2021

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.

@Karsten1987
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants