Closed alexdunnjpl closed 1 year ago
@alexdunnjpl can we fix this, as well as any of the other flake issues that are occurring so we can get this CI working correctly? We will not be able to tag a release until this can be built cleanly.
@jordanpadams roger that, will sort it out today.
I'll skip these tests for CI and disable the lint, and open icebox'd tickets for each of them.
After that, for the linting, do you want the codebase brought into spec for the existing lint profile, or the lint profile adjusted to disable the new warnings, or is fixing it a lower priority than closing out other tickets?
@jordanpadams what's the level of urgency on this?
To sort this out properly requires:
tox lint
runs all unit tests - working on splitting that out now but I have to learn how2tox first)Basically I want to know if it can wait until next week or not - it's not a lot of work, mind.
nvm, we're good
@alexdunnjpl just wanted to verify, does any of your changes to the tox / flake8 rules above conflict with what we have here: https://github.com/NASA-PDS/template-repo-python
the premise here is we want to ensure we are consistent across python repos. if we need to change something here, I want to make sure this is agreed upon with @nutjob4life and a PR is created on the template repo to ensure we use that config moving forward (and probably need to rollout to existing python repos)
@jordanpadams
The flake8 ignores as-is are a superset of the desirable flake8 ignores for this project, which is the template set plus W503 - see L163 here
Based on the comment there, it seems like W503 should be added to the template.
Regarding the others, they're not desirable to keep long-term, but the alternative until the underlying style issues are fixed is to have the tests fail, removing the ability of the tests to enforce good style for any new contributions. So adding the bunch of temporary ignores is the least-worst option here.
W503 should be added to the template
Yes yes yes.
Human perception studies (amongst left-to-right readers) show that people tend to scan for significance on the left sides of things, making the so-called "F" pattern for reading and skimming. W503's placement of operators on the right is wrong-headed.
Looking at what that rule actually is, MAAASSIVE +1
Will open a PR on the template repo for that now.
🐛 Describe the bug
349 was merged despite failing unit tests
📜 To Reproduce
See relevant branch integration test report
🕵️ Expected behavior
Branch integration tests should pass
⚙️ Engineering Details
Suggested resolution
flake8
lint in BIT Github Action, as the flood of failures hide non-trivial BIT failures.@jordanpadams let me know if you want me to tackle this, I know @ramesh-maddegoda has a lot going on atm.