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-39785: Package modernization and ruff #162
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #162 +/- ##
==========================================
- Coverage 93.94% 93.88% -0.07%
==========================================
Files 43 43
Lines 2826 2830 +4
==========================================
+ Hits 2655 2657 +2
- Misses 171 173 +2
☔ View full report in Codecov by Sentry. |
E402 triggers if `from __future__` is before the package docstring. Moving the package docstring above every import appeases ruff.
This makes it clear that we do not expect the variable to be used.
This was found by bugbear: B010: Do not call `setattr` with a constant attribute value. It is not any safer than normal property access.
This matches the flake8 and pydocstyle configuration that is already in use.
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.
Looks good, few minor comments.
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v3 | ||
- uses: chartboost/ruff-action@v1 |
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.
Should we be adding a reusable workflow for ruff to rubin_workflows
?
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.
Yes, although I don't think we can add it to the reusable linter workflow so we'd have to make a standalone workflow with those four lines in it. Maybe a shared linter could include ruff but not run it if a use_ruff parameter is false and then we can add a "use_ruff = True" argument to that. I'm happy to fix things up based on the RFC.
Checklist
doc/changes