Skip to content
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

Merged
merged 5 commits into from
Aug 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions contributing/pull_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,18 +249,17 @@ The best way to contribute is to fork the CodeIgniter4 repository, and "clone" t
7. Fix existing bugs on the [Issue tracker](https://github.com/codeigniter4/CodeIgniter4/issues) after confirming that no one else is working on them.
8. [Commit](https://help.github.com/en/desktop/contributing-to-projects/committing-and-reviewing-changes-to-your-project) the changed files in your contribution branch.
- `> git commit`
- Commit messages are expected to be descriptive of what you changed specifically. Commit messages like "Fixes #1234" would be asked by the reviewer to be revised.
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.
Copy link
Member

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sent #6446

- `> composer analyze`
- `> vendor/bin/rector process`
11. Run unit tests on the specific file you modified. If there are no existing tests yet, please create one.
10. Run unit tests on the specific file you modified. If there are no existing tests yet, please create one.
- `> vendor/bin/phpunit tests/system/path/to/file/you/modified`
- Make sure the tests pass to have a higher chance of merging.
12. [Push](https://docs.github.com/en/github/using-git/pushing-commits-to-a-remote-repository) your contribution branch to your fork.
11. [Push](https://docs.github.com/en/github/using-git/pushing-commits-to-a-remote-repository) your contribution branch to your fork.
- `> git push origin <new-branch-name>`
13. Send a [pull request](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork).
14. Label your pull request with the appropriate label if you can.
12. Send a [pull request](https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/creating-a-pull-request-from-a-fork).
13. Label your pull request with the appropriate label if you can.

See [Contribution workflow](./workflow.md) for Git workflow details.

Expand Down
10 changes: 1 addition & 9 deletions contributing/signing.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,4 @@ bash shell to use the **-S** option to force the secure signing.
## Commit Messages

Regardless of how you sign a commit, commit messages are important too.
They communicate the intent of a specific change, concisely. They make
it easier to review code, and to find out why a change was made if the
code history is examined later.

The audience for your commit messages will be the codebase maintainers,
any code reviewers, and debuggers trying to figure out when a bug might
have been introduced.

Make your commit messages meaningful.
See [Contribution Workflow](./workflow.md#commit-messages) for details.
36 changes: 34 additions & 2 deletions contributing/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,16 +139,48 @@ Your local changes need to be *committed* to save them in your local
repository. This is where [contribution signing](./signing.md) comes
in.

Now we don't have detailed rules on commits and its messages. But
[atomic commit](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) is recommended.
Keep your commits atomic. One commit for one change.

There are some references for writing good commit messages:

- [Git Best Practices — AFTER Technique - DZone DevOps](https://dzone.com/articles/git-best-practices-after-technique-1)
- [Semantic Commit Messages](https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716)
- [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/)

If there are intermediate commits that are not meaningful to the overall PR,
such as "Fix error on style guide", "Fix phpstan error", "Fix mistake in code",
and other related commits, you can squash your commits so that we can have a clean commit history.
But it is not a must.

### Commit Messages

Commit messages are important. They communicate the intent of a specific change, concisely.
They make it easier to review code, and to find out why a change was made
if the code history is examined later.

The audience for your commit messages will be the codebase maintainers,
any code reviewers, and debuggers trying to figure out when a bug might
have been introduced.

Make your commit messages meaningful.

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.

You can have as many commits in a branch as you need to "get it right".
For instance, to commit your work from a debugging session:

```console
> git add .
> git commit -S -m "Find and fix the broken reference problem"
> git commit -S -m "Fix the broken reference problem"
```

Just make sure that your commits in a feature branch are all related.

### When you work on two features

If you are working on two features at a time, then you will want to
switch between them to keep the contributions separate. For instance:

Expand All @@ -164,7 +196,7 @@ switch between them to keep the contributions separate. For instance:
> git switch develop
```

The last checkout makes sure that you end up in your *develop* branch as
The last switch makes sure that you end up in your *develop* branch as
a starting point for your next session working with your repository.
This is a good practice, as it is not always obvious which branch you
are working in.
Expand Down