From e215d66d1b82905cda814b7155cfebd9d9b15305 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 28 Aug 2022 11:11:40 +0900 Subject: [PATCH 1/5] docs: remove description on squashing commits It is not so important thing. --- contributing/pull_request.md | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/contributing/pull_request.md b/contributing/pull_request.md index 628d5593da5a..8a944ad1f490 100644 --- a/contributing/pull_request.md +++ b/contributing/pull_request.md @@ -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. +9. If you have touched PHP code, run static analysis. - `> 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 ` -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. From 122133bdb3db78578fbecbaa88b89340e023d075 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 28 Aug 2022 11:13:28 +0900 Subject: [PATCH 2/5] docs: add explanation on commits and commit messages --- contributing/workflow.md | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/contributing/workflow.md b/contributing/workflow.md index 7c7c1ca7cc21..7e4809c83817 100644 --- a/contributing/workflow.md +++ b/contributing/workflow.md @@ -139,16 +139,38 @@ 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 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: From 55fd7f0239293b4e1201ffa6048a8a49924991ce Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 28 Aug 2022 11:14:40 +0900 Subject: [PATCH 3/5] docs: update wording We changed `git checkout` to `git switch` before. --- contributing/workflow.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributing/workflow.md b/contributing/workflow.md index 7e4809c83817..12ab737eb5f8 100644 --- a/contributing/workflow.md +++ b/contributing/workflow.md @@ -186,7 +186,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. From 2e956258170612d8d83c340a71cbd37e2525ed2b Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 28 Aug 2022 12:50:52 +0900 Subject: [PATCH 4/5] docs: move description from signing to workflow --- contributing/signing.md | 10 +--------- contributing/workflow.md | 12 +++++++++++- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/contributing/signing.md b/contributing/signing.md index ca84ef79a9a7..d312d3fc217f 100644 --- a/contributing/signing.md +++ b/contributing/signing.md @@ -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. diff --git a/contributing/workflow.md b/contributing/workflow.md index 12ab737eb5f8..50673ce10166 100644 --- a/contributing/workflow.md +++ b/contributing/workflow.md @@ -154,7 +154,17 @@ 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 + +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. From c3820963f6ad75836ddcd8c3bc5c1e6591f24096 Mon Sep 17 00:00:00 2001 From: kenjis Date: Sun, 28 Aug 2022 12:51:18 +0900 Subject: [PATCH 5/5] docs: add link to detailed explanation --- contributing/pull_request.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributing/pull_request.md b/contributing/pull_request.md index 8a944ad1f490..ed6d0dfde9e6 100644 --- a/contributing/pull_request.md +++ b/contributing/pull_request.md @@ -249,7 +249,7 @@ 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 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. + - 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. - `> composer analyze` - `> vendor/bin/rector process`