-
Notifications
You must be signed in to change notification settings - Fork 12
Add pre-commit config #191
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
base: main
Are you sure you want to change the base?
Conversation
| quiet-level = 3 | ||
|
|
||
| [tool.ruff] | ||
| line-length = 120 |
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.
I am fine with any value here and don't want this to become a point for bikeshedding this PR. I chose 120 for now to make the diff as small as possible (the fewer changes, the easier to review). If anyone has a preference on what this value should be, I recommend we defer that discussion to a follow-up PR.
| import pyarrow.parquet as pq | ||
|
|
||
| from .common_fixtures import setup_and_teardown, get_all_parquet_relative_file_paths | ||
| from .common_fixtures import get_all_parquet_relative_file_paths |
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.
Can you please verify that the tests are passing (the CI for this has not yet been added. Related issue: #173)?
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.
I would strongly support solving #173 before we merge this.
|
I'm going to cut the scope of this so we can start adding basic CI checks. I'll keep this to only whitespace/end-of-file fixes and copyright fixes for now. |
- Resolved merge conflicts between shellcheck fixes and upstream SCRIPT_DIR changes - Fixed SC2086 (quote variables) in new slurm scripts - Fixed SC2155 (declare and assign separately) - Fixed SC2129 (group redirects) - Added missing copyright headers
This adds a
.pre-commit-config.yamlto get this repo up to speed with NVIDIA copyright requirements and the bare minimum of code style enforcement.As a follow-up we can enable CI checks to enforce these.