-
Notifications
You must be signed in to change notification settings - Fork 11
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
DM-38499: Clean up package and change to run flake8/ruff #115
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
=======================================
Coverage 87.50% 87.50%
=======================================
Files 5 5
Lines 32 32
Branches 2 2
=======================================
Hits 28 28
Misses 2 2
Partials 2 2
☔ View full report in Codecov by Sentry. |
1874e05
to
dd27a7d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few comments to start with. Still need to read through installation, scripts, state, and tests.
.pre-commit-config.yaml
Outdated
# supported by your project here, or alternatively use | ||
# pre-commit's default_language_version, see | ||
# https://pre-commit.com/#top_level-default_language_version | ||
language_version: python3.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If latest is python3.11, how come this is python3.10?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Formally we are still on python 3.10 although if this isn't merging until python 3.11 is the standard then I can change that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see you're reading "supported" differently from the way I did (the highest level in your matrix).
No need for it to be checking for the python2 config parser.
ruff is preferred over flake8 but will only be used if a ruff configuration is found. No attempt is made to check that ruff or flake8 are present in the path.
This allows the variables to be inlined in the large string.
tomllib is standard package with python 3.11.
black broke it up because of a trailing comma but there is no need for it.
This makes it easier to run on jenkins with python 3.10.
sconsUtils only has to work with current rubin-env
miniconda is broken on python 3.11 + macOS
Should not be merged until we update to rubin-env 7 so that we have the fast flake8 and ruff available. Potentially this PR could be split into two: one for running flake8/ruff and the other all the cleanups.