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.

Sign up for GitHub

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

Jump to bottom

Duplicate CI Linting jobs #479

Closed
jaron-l opened this issue Jul 28, 2021 · 1 comment · Fixed by #480
Closed

Duplicate CI Linting jobs #479

jaron-l opened this issue Jul 28, 2021 · 1 comment · Fixed by #480

Comments

@jaron-l
Copy link
Member

jaron-l commented Jul 28, 2021 •
edited
Loading

As brought up in #473, lint.yml and formal.yml GitHub workflows are redundant in that they both perform linting tasks.

Format.yml was added as it used pre-commit while lint.yml uses ament linting packages. Format.yml (pre-commit) has been set up to use most of the ament_linting packages that the original workflow uses. With the exception of ament_xmllint, ament_flake8, and ament_pep257, format.yml uses the packages exactly as lint.yml. For ament_flake8 and ament_pep257, format.yml just uses the sub-packages directly (flake8 and pep257 respectively) rather than using the ament variants.

Paths forward:

  1. Should we just remove the duplicate jobs in lint.yml? (I would probably just reduce it down to ament_xmllint as I don't know if there's an added benefit of ament_flake8 and ament_pep257 beyond what's already in pre-commit now).
  2. Should I try to move the ament_xmllint job into the pre-commit config so that it has the added benefit of pre-commit as well as being in the job?
@jaron-l
Copy link
Member Author

jaron-l commented Jul 29, 2021

When I further investigated, it looks like there is a xml linter in the built-in/standard pre-commit. So it looks like beyond the fact that 3 packages were switched for equivalents, format.yml does everything and more than lint.yml so lint.yml probably can just be deleted.

jaron-l added a commit to jaron-l/ros2_control that referenced this issue Jul 29, 2021
@jaron-l
ci: 💚 Remove extra linting CI workflow as it is redudant
573e3a0 
When the pre-commit workflow was added in ros-controls#473,
the GitHub workflow specified in lint.yml became obsolete
as all linting in now done through pre-commit in format.yml.
With the exception of three packages,
all linting packages have an exact match:
1.ament_pep257 -> pypi pep257
2. ament_flake8 -> pypi flake8
3. ament_xmllint -> pre-commit xml linting

Fixes Duplicate CI Linting jobs ros-controls#479
jaron-l added a commit to jaron-l/ros2_control that referenced this issue Jul 29, 2021
@jaron-l
ci: 💚 Remove extra linting CI workflow
16801e1 
When the pre-commit workflow was added in ros-controls#473,
the GitHub workflow specified in lint.yml became obsolete
as all linting in now done through pre-commit in format.yml.
With the exception of three packages,
all linting packages have an exact match:
1.ament_pep257 -> pypi pep257
2. ament_flake8 -> pypi flake8
3. ament_xmllint -> pre-commit xml linting

Fixes Duplicate CI Linting jobs ros-controls#479
@jaron-l jaron-l mentioned this issue Jul 29, 2021
Merged
pac48 pushed a commit to pac48/ros2_control that referenced this issue Jan 26, 2024
@moriarty @bmagyar
Don't run humble workflows from master branch (ros-controls#479) (ros…
64fd333 
…-controls#622)

* Remove workflows for distros not supported on master

* Revert "Remove workflows for distros not supported on master"

This reverts commit e916af3b19ba8642312692328ebc71a2f92ac93c.

* Don't run humble workflows on master

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ci: Remove extra linting CI workflow jaron-l/ros2_control
1 participant
@jaron-l