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

enable code linting check for all supported Python versions #3725

Merged
merged 8 commits into from
Feb 20, 2022

Conversation

JensTimmerman
Copy link
Contributor

run linting tests for all supported python versions

@JensTimmerman
Copy link
Contributor Author

seems to work, tests fail because we're now catching errors:

./easybuild/tools/configobj.py:2026:55: F812 list comprehension redefines 'line' from line 2014

@akesandgren
Copy link
Contributor

@boegel This looks ok to me, and the caught error might need to be fixed ....

@boegel boegel modified the milestones: 4.4.2, release after 4.4.2 Sep 1, 2021
@boegel boegel modified the milestones: 4.5.0, 4.x Oct 13, 2021
@akesandgren
Copy link
Contributor

@JensTimmerman could you do "eb --sync-pr-with-develop 3725 --pr-target-repo easybuild-framework" on this one to make sure it's up to date.

@JensTimmerman
Copy link
Contributor Author

@akesandgren that didn't seem to work for me, I just merged develop into this branch, if that also works for you?

@akesandgren
Copy link
Contributor

@JensTimmerman could you do "eb --sync-pr-with-develop 3725 --pr-target-repo easybuild-framework" on this one to make sure it's up to date.

It did exactly what it needed to do, make sure that it applies on top of current develop HEAD and re-kick the test suite.
Adding the commit for 3.10 would have caused the same thing I was after, i.e. re-running the tests.

You could also add a small fix for the line redefined problem it detects, not sure what @boegel would accept here but I think
easybuild/tools/configobj.py:2026 can be changed to use "ln" instead of "line" without too much complaints from him...

JensTimmerman and others added 2 commits February 14, 2022 17:03
py3 functions will always fail in py2, so ignore for linting for now. c
@@ -18,4 +22,4 @@ jobs:

- name: Run flake8 to verify PEP8-compliance of Python code
run: |
flake8
flake8 --exclude ./easybuild/tools/py2vs3/py3.py
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally this only get's skipped for python2, but I couldn't quickly find a good filter for this case.

Copy link
Member

Choose a reason for hiding this comment

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

fixed in a2ec58b

@boegel boegel modified the milestones: 4.x, next release (4.5.4?) Feb 19, 2022
@boegel boegel changed the title Enable static analysis for python2.7 (tests fail because broken code is identified) enable code linting check for all supported Python versions Feb 19, 2022
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel
Copy link
Member

boegel commented Feb 20, 2022

Thanks a lot @JensTimmerman!

@boegel boegel merged commit 8470097 into easybuilders:develop Feb 20, 2022
boegel added a commit to boegel/easybuild-framework that referenced this pull request Mar 1, 2022
enable code linting check for all supported Python versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants