-
Notifications
You must be signed in to change notification settings - Fork 300
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
Add basic pre-commit config, apply fixes and run it in CI #292
Conversation
This is a misleading PR title; it suggests no changes to the actual code. I don’t have a problem with that aspect of the PR contents, but please do try to employ accurate PR titles. @IIITM-Jay please review. |
To avoid an enormouse commit, pylint and pyright are commented out for now.
3e1dbb0
to
3ba18d3
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.
Thanks to the both of you.
@Timmmm @IIITM-Jay After merging, a post-merge check is failing, probably because of a formatting error that has been inserted since this PR was created. Can either of you look into it and file a PR to get things passing again? https://github.com/riscv/riscv-opcodes/actions/runs/11225349353/job/31203877551 |
Ah probably a race with another PR. I'll fix it! |
Thank you! |
This little bit of code from riscv#268 was not formatted in riscv#292 because it was not merged yet. Basically it was a semantic conflict between the PRs, but not a text conflict, so Git can't detect it. This can be prevented using merge queues, but it probably isn't necessary on a repo of this size.
#296 should fix it. |
Add a basic pre-commit config to format Python files using Black, and run it in CI. The second commit in this MR contains the actual formatting fixes.
I will fix the Pylint failures and add types in future MRs so that they can be enabled.