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

Fix exclude regression #387

Merged
merged 3 commits into from
Jun 13, 2022
Merged

Fix exclude regression #387

merged 3 commits into from
Jun 13, 2022

Conversation

methylDragon
Copy link
Contributor

@methylDragon methylDragon commented Jun 9, 2022

This PR fixes #382 by explicitly adding any exclude args in the flake8 config file (normally configuration/ament_flake8.ini) to the exclude arglist.

Notably, this uses the same parsing logic that the version of flake8 ROS is using on focal and jammy (3.9.7 and 4.0.0).
I tried to account for differences in the parsing logic between 3.9.7 and 4.0.0 with a try except clause.

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Copy link
Contributor Author

Demonstration of fix, with flake8 lint errors in listener*.py and talker*.py

image

@methylDragon methylDragon force-pushed the ch3/fix_exclude_regression branch 2 times, most recently from 1a9e4ab to 6915bc6 Compare June 9, 2022 23:15
@methylDragon
Copy link
Contributor Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@methylDragon
Copy link
Contributor Author

methylDragon commented Jun 10, 2022

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Single quotes!! ✊ ✊ ✊

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Copy link
Contributor Author

Unstable windows build is unrelated to this PR

Copy link
Contributor

@aprotyas aprotyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small nit but approving regardless. I think the only thing that would change if you initialized the OptionManager for flake8-4.x would be the order in which you try-except with the aggregator.

def parse_config_file(config_file):
from flake8.options import config, manager, aggregator

opts_manager = manager.OptionManager(prog='flake8', version='3.0.0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why 3.x? In Ubuntu 22.04 the default flake8 version is 4.x.

@methylDragon
Copy link
Contributor Author

methylDragon commented Jun 11, 2022

I've reordered the try-except.

Testing on both jammy and focal since it matters due to the API breaking change for flake8.

Jammy:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Focal:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

Signed-off-by: methylDragon <methylDragon@gmail.com>
@methylDragon
Copy link
Contributor Author

Unstable focal aarch64 build is unrelated to this PR

@methylDragon methylDragon merged commit 66c76cc into master Jun 13, 2022
@methylDragon methylDragon deleted the ch3/fix_exclude_regression branch June 13, 2022 04:43
emersonknapp pushed a commit to emersonknapp/ament_lint that referenced this pull request Sep 14, 2023
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
emersonknapp pushed a commit to emersonknapp/ament_lint that referenced this pull request Sep 14, 2023
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
clalancette added a commit that referenced this pull request Sep 20, 2023
#451)

* Fix exclude regression (#387)

* Fix compatibility with flake8 version 5 (#410)

* Fix compatibility with flake8 version 5

The ConfigFileFinder class no longer exists in flake8 version 5.
The get_style_guide() code has been updated from the latest
api.legacy.get_style_guide() in flake8.

Signed-off-by: Timo Röhling <roehling@debian.org>
Co-authored-by: Chris Lalancette <clalancette@gmail.com>
Signed-off-by: Emerson Knapp <emerson.b.knapp@gmail.com>
Co-authored-by: methylDragon <methylDragon@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ament_flake8] Exclude directories / file patterns defined in config file are ignored
2 participants