-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
docs: add about PR commits #6437
Conversation
It is not so important thing.
We changed `git checkout` to `git switch` before.
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.
Good foundational practices!
9. If there are intermediate commits that are not meaningful to the overall PR, such as "Fixed error on style guide", "Fixed phpstan error", "Fixing mistake in code", and other related commits, it is advised to squash your commits so that we can have a clean commit history. | ||
10. If you have touched PHP code, run static analysis. | ||
- Commit messages are expected to be descriptive of why and what you changed specifically. Commit messages like "Fixes #1234" would be asked by the reviewer to be revised. [Atomic commit](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) is recommended. See [Contribution Workflow](./workflow.md#commit-messages) for details. | ||
9. If you have touched PHP code, run static analysis. |
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.
We should update composer analyze
to dry-run Rector as well, since that is part of our pipeline. I also just realized we aren't using Psalm here! Since you introduced me to it I've added it to all my libraries, and it frequently catches different things from PHPStan (especially around typing). I can look into 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.
I sent #6446
Now that you've created a good guide to writing commits, I thought it would be helpful to add how to edit wrong commits. |
Description
Big commits and/or inappropriate commit messages make the reviews difficult.
Checklist: