-
Notifications
You must be signed in to change notification settings - Fork 84
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
Configure pre-commit hooks and GitHub Actions #95
Conversation
ea0094c
to
7def28f
Compare
I noticed that some files are currently blocked by the pre-commit hooks. To resolve these issues and ensure a smooth merge, I have made the necessary changes in separate pull requests:
Please review and merge these pull requests first. Once they are merged, the pre-commit hooks should pass successfully for this pull request. If there are any further issues or if you have any suggestions for improving the pre-commit hooks or GitHub Actions workflow, please let me know. I'm open to feedback and collaboration to enhance our development process. Thank you for your attention to code quality and for setting up these valuable tools! |
912c745
to
ed6e1e8
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 for introducing this linter tool to the workflow. It looks awesome!
I have a question, is there a way to show the code diff in the workflow log output? Currently I cannot find the diff in this workflow log.
I don't expect many contributors would download pre-commit
tool and run it locally. Of course, we can add some instructions in CONTRIBUTING.md
to guide them, but it still creates additional barriers for them. So relying on the workflow logs may be an easier way for them. Well, if contributors are really stuck in passing the linter check, in the end, as maintainers we may have to step up to help them, e.g. show them the mistake in code review, or even commit a fix for them.
Also, we may add some more info in the CONTRIBUTING.md
file, to ask contributors follow the Google C++ coding style guide, and the corresponding Python coding style guide (is it https://peps.python.org/pep-0008 ?).
The pre-commit hooks failed with exit code 143 when executing clang-format on large files, as seen in the GitHub Actions logs:
This issue is likely caused by a crash due to insufficient resources, as discussed in actions/runner-images#6680. To resolve this problem, I will:
By excluding the large files, we can ensure that the pre-commit hooks run successfully without encountering resource limitations. |
Thank you for sharing your thoughts and concerns regarding the pre-commit hooks and contributor experience. I agree that we shouldn't force every contributor to use pre-commit locally, as it may create additional barriers for them. To make the process easier, I will enable verbosity for all pre-commit hooks in the GitHub Actions workflow. This will display the code diff and highlight the specific areas where the code fails to meet the style guidelines. Contributors can then refer to the workflow logs to identify and fix the issues without needing to run pre-commit locally. Regarding the coding style guides, I suggest we update the
By providing clear guidelines in the I appreciate your input and collaboration in improving the contributor experience. |
That sounds good! Thanks for the efforts! |
0d159a5
to
c68bcdf
Compare
After rebasing, the fiels not modified are excluded from pre-commit. |
This was my mistaken. |
8e5cd12
to
bf7f775
Compare
With merging #100 and #101, the pre-commit successed. |
6112fab
to
067c135
Compare
844c7fd
to
336a16c
Compare
@HenryRLee I've made the following changes based on your review:
Please let me know if you have any further suggestions or concerns. I'm happy to improve the contribution experience for everyone involved. |
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 great! The guidelines are very detail. Many thanks for your efforts!
This Pull Request introduces several changes to improve the project's code quality assurance processes and refactor the CI workflows.
Python Formatting and Linting
Black
andisort
and addedRuff
for Python linting/formattingpyproject.toml
C++ Formatting
.clang-format
for C++ formatting based on Google style (discussed on Apply code formatting to C and C++ files #97)Pre-commit Hooks
.pre-commit-config.yaml
GitHub Actions Refactoring
ci.yml
actions/checkout
andactions/setup-python
pre-commit.yml
to run pre-commit checks in CIdependabot.yml
to keep GitHub Action versions up-to-dateBenefits
These changes offer several benefits:
Developers can run the pre-commit hooks locally to identify and fix problems before committing, leading to a smoother development experience and a more stable codebase.
Please review the changes and provide any feedback or suggestions. Let's work together to enhance our code quality and streamline our development process!