Skip to content

Commit 46b301f

Browse files
authored
Merge pull request #8474 from kenjis/docs-contrib-pr
docs: update contributing/pull_request.md
2 parents 9c669c3 + d117b8a commit 46b301f

3 files changed

Lines changed: 141 additions & 91 deletions

File tree

contributing/pull_request.md

Lines changed: 98 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,27 @@
22

33
## Contributions
44

5-
We expect all contributions to conform to our
6-
[style guide](https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/styleguide.md),
7-
be commented (inside the PHP source files), be documented (in the
8-
[user guide](https://codeigniter4.github.io/userguide/)), and unit tested (in
9-
the [test folder](https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests)).
5+
We expect all contributions to
6+
- conform to our [style guide](./styleguide.md),
7+
- be commented (inside the PHP source files),
8+
- be documented (in the [user guide](https://codeigniter4.github.io/userguide/)),
9+
- and unit tested (in the [test folder](https://github.com/codeigniter4/CodeIgniter4/tree/develop/tests)).
1010

11-
Note, we expect all code changes or bug-fixes to be accompanied by one or more tests added to our test suite
12-
to prove the code works. If pull requests are not accompanied by relevant tests, they will likely be closed.
13-
Since we are a team of volunteers, we don't have any more time to work on the framework than you do. Please
14-
make it as painless for your contributions to be included as possible. If you need help with getting tests
15-
running on your local machines, ask for help on the forums. We would be happy to help out.
11+
> [!IMPORTANT]
12+
> We expect all code changes or bug-fixes to be accompanied by one or more tests
13+
> added to our test suite to prove the code works.
1614
17-
The [Open Source Guide](https://opensource.guide/) is a good first read for those new to contributing to open source!
15+
If pull requests are not accompanied by relevant tests, they will likely be closed.
16+
Since we are a team of volunteers, we don't have any more time to work on the
17+
framework than you do. Please make it as painless for your contributions to be
18+
included as possible.
19+
20+
If you need help with getting tests running on your local machines, ask for help
21+
on the [forum](https://forum.codeigniter.com/forumdisplay.php?fid=27). We would
22+
be happy to help out.
23+
24+
The [Open Source Guide](https://opensource.guide/) is a good first read for those
25+
new to contributing to open source!
1826

1927
## CodeIgniter Internals Overview
2028

@@ -29,7 +37,8 @@ Your Pull Requests (PRs) need to meet our guidelines.
2937

3038
If your Pull Requests fail to pass these guidelines, they will be declined,
3139
and you will need to re-submit when you've made the changes.
32-
This might sound a bit tough, but it is required for us to maintain the quality of the codebase.
40+
This might sound a bit tough, but it is required for us to maintain the quality
41+
of the codebase.
3342

3443
### PHP Style
3544

@@ -38,10 +47,11 @@ This might sound a bit tough, but it is required for us to maintain the quality
3847
All code must conform to our [Style Guide](./styleguide.md), which is
3948
based on PSR-12.
4049

41-
This makes certain that all submitted code is of the same format
42-
as the existing code and ensures that the codebase will be as readable as possible.
50+
This makes certain that all submitted code is of the same format as the existing
51+
code and ensures that the codebase will be as readable as possible.
4352

44-
You can fix most of the coding style violations by running this command in your terminal:
53+
You can fix most of the coding style violations by running this command in your
54+
terminal:
4555

4656
```console
4757
composer cs-fix
@@ -55,7 +65,8 @@ composer cs
5565

5666
### Unit Testing
5767

58-
If you are not familiar with Unit Testing, see [the forum thread](https://forum.codeigniter.com/showthread.php?tid=81830).
68+
If you are not familiar with Unit Testing, see
69+
[the forum thread](https://forum.codeigniter.com/showthread.php?tid=81830).
5970

6071
Unit testing is expected for all CodeIgniter components. We use PHPUnit,
6172
and run unit tests using GitHub Actions for each PR submitted or changed.
@@ -93,7 +104,7 @@ See the following for more information.
93104

94105
Do not add comments that are superficial, duplicated, or stating the obvious.
95106

96-
### Documentation
107+
### User Guide
97108

98109
The User Guide is an essential component of the CodeIgniter framework.
99110

@@ -106,18 +117,21 @@ then you will need to add to the documentation.
106117
New classes, methods, parameters, changing default values, changing behavior etc.
107118
are all changes that require a change to documentation.
108119

109-
Also, the [Changelog](https://codeigniter4.github.io/CodeIgniter4/changelogs/index.html) must be updated for every change,
110-
and [PHPDoc](https://github.com/codeigniter4/CodeIgniter4/blob/develop/phpdoc.dist.xml) blocks must be maintained.
120+
Also, the [Changelog](https://codeigniter4.github.io/CodeIgniter4/changelogs/index.html)
121+
must be updated for every change, and PHPDoc blocks must be maintained.
111122

112123
See [Writing CodeIgniter Documentation](./documentation.rst).
113124

114125
#### Changelog
115126

116-
The [Changelog](https://codeigniter4.github.io/CodeIgniter4/changelogs/index.html), in the user guide, needs to be kept up-to-date. Not
117-
all changes will need an entry in it, but the following items should.
127+
The [Changelog](https://codeigniter4.github.io/CodeIgniter4/changelogs/index.html)
128+
in the [user guide](https://github.com/codeigniter4/CodeIgniter4/tree/develop/user_guide_src/source/changelogs)
129+
needs to be kept up-to-date. Not all changes will need an entry in it, but the
130+
following items should.
118131

119132
- all breaking changes (BCs)
120133
- all enhancements (new features, new classes, new APIs)
134+
- message changes
121135
- other behavior changes
122136
- deprecations
123137
- major bug fixes
@@ -127,11 +141,12 @@ all changes will need an entry in it, but the following items should.
127141
If your PR requires users to do something when they upgrade CodeIgniter, or
128142
changes the Config values,
129143
the [Upgrading Guide](https://codeigniter4.github.io/CodeIgniter4/installation/upgrading.html)
130-
is also needed.
144+
in the [installation](https://github.com/codeigniter4/CodeIgniter4/tree/develop/user_guide_src/source/installation)
145+
folder is also needed.
131146

132147
- Add an instruction what to do when upgrading.
133148
- If you add new properties or changes default values in Config files, add the
134-
changes in *"Project Files > Content Changes > Config"*.
149+
changes in *"Project Files > Content Changes > Config"*.
135150

136151
### CSS
137152

@@ -154,12 +169,19 @@ break with earlier versions of the framework.
154169

155170
#### Breaking Changes
156171

157-
In general, any change that would disrupt existing uses of the framework is considered a "Breaking Change" (BC) and will not be favorably considered. A few specific examples to pay attention to:
172+
In general, any change that would disrupt existing uses of the framework is
173+
considered a "Breaking Change" (BC) and will not be favorably considered. A few
174+
specific examples to pay attention to:
158175

159-
1. New classes/properties/constants in `system` are acceptable, but anything in the `app` directory that will be used in `system` should be backwards-compatible.
160-
2. Any changes to non-private methods must be backwards-compatible with the original definition.
161-
3. Deleting non-private properties or methods without prior deprecation notices is frowned upon and will likely be closed.
162-
4. Deleting or renaming public classes and interfaces, as well as those not marked as `@internal`, without prior deprecation notices or not providing fallback solutions will also not be favorably considered.
176+
1. New classes/properties/constants in `system` are acceptable, but anything in
177+
the `app` directory that will be used in `system` should be backwards-compatible.
178+
2. Any changes to non-private methods must be backwards-compatible with the original
179+
definition.
180+
3. Deleting non-private properties or methods without prior deprecation notices
181+
is frowned upon and will likely be closed.
182+
4. Deleting or renaming public classes and interfaces, as well as those not marked
183+
as `@internal`, without prior deprecation notices or not providing fallback
184+
solutions will also not be favorably considered.
163185

164186
### Mergeability
165187

@@ -174,48 +196,57 @@ working on your contribution.
174196

175197
### Branching
176198

177-
All bug fixes should be sent to the __"develop"__ branch, this is where the next bug fix version will be developed.
199+
All bug fixes should be sent to the __"develop"__ branch, this is where the next
200+
bug fix version will be developed.
178201

179-
PRs with any enhancement should be sent to next minor version branch, e.g. __"4.3"__
202+
PRs with any enhancement should be sent to next minor version branch, e.g. __"4.5"__
180203

181-
The __"master"__ branch will always contain the latest stable version and is kept clean so a "hotfix" (e.g. an
182-
emergency security patch) can be applied to the "master" branch to create a new version, without worrying
183-
about other features holding it up. Any sent to the "master" branch will be closed automatically.
204+
The __"master"__ branch will always contain the latest stable version and is kept
205+
clean so a "hotfix" (e.g. an emergency security patch) can be applied to the
206+
"master" branch to create a new version, without worrying about other features
207+
holding it up. Any sent to the "master" branch will be closed automatically.
184208

185209
If you have multiple changes to submit,
186210
please place all changes into their own branch on your fork.
187211

188-
**One thing at a time:** A pull request should only contain one change. That does not mean only one commit,
189-
but one change - however many commits it took. The reason for this is that if you change X and Y,
190-
but send a pull request for both at the same time, we might really want X but disagree with Y,
191-
meaning we cannot merge the request. Using the Git-Flow branching model you can create new
192-
branches for both of these features and send two requests.
212+
**One thing at a time:** A pull request should only contain one change. That does
213+
not mean only one commit, but one change - however many commits it took. The reason
214+
for this is that if you change X and Y, but send a pull request for both at the
215+
same time, we might really want X but disagree with Y, meaning we cannot merge
216+
the request. Using the Git-Flow branching model you can create new branches for
217+
both of these features and send two requests.
193218

194-
A reminder: **please use separate branches for each of your PRs** - it will make it easier for you to keep
195-
changes separate from each other and from whatever else you are doing with your repository!
219+
> [!IMPORTANT]
220+
> **Please use separate branches for each of your PRs** - it will make it easier
221+
> for you to keep changes separate from each other and from whatever else you are
222+
> doing with your repository!
196223
197224
### Signing
198225

199226
You must [GPG-sign](./signing.md) your work, certifying that you either wrote the work or
200-
otherwise have the right to pass it on to an open-source project. See [Developer's Certificate of Origin](./DCO.md).
227+
otherwise have the right to pass it on to an open-source project. See
228+
[Developer's Certificate of Origin](./DCO.md).
201229

202230
This is *not* just a "signed-off-by" commit, but instead, a digitally signed one.
203231

204232
See [Contribution signing](./signing.md) for details.
205233

206234
### Static Analysis on PHP code
207235

208-
We cannot, at all times, guarantee that all PHP code submitted on pull requests to be working well without
209-
actually running the code. For this reason, we make use of two static analysis tools, [PHPStan][1]
210-
and [Rector][2] to do the analysis for us.
236+
We cannot, at all times, guarantee that all PHP code submitted on pull requests
237+
to be working well without actually running the code. For this reason, we make
238+
use of two static analysis tools, [PHPStan][1] and [Rector][2] to do the analysis
239+
for us.
211240

212-
These tools have already been integrated into our CI/CD workflow to minimize unannounced bugs. Pull requests
213-
are expected that their code will pass these two. In your local machine, you can manually run these tools
214-
so that you can fix whatever errors that pop up with your submission.
241+
These tools have already been integrated into our CI/CD workflow to minimize
242+
unannounced bugs. Pull requests are expected that their code will pass these two.
243+
In your local machine, you can manually run these tools so that you can fix
244+
whatever errors that pop up with your submission.
215245

216246
#### PHPStan
217247

218-
PHPStan is expected to scan the entire framework by running this command in your terminal:
248+
PHPStan is expected to scan the entire framework by running this command in your
249+
terminal:
219250

220251
```console
221252
vendor/bin/phpstan analyse
@@ -252,7 +283,10 @@ vendor/bin/rector process path/to/file
252283

253284
## How-to Guide
254285

255-
The best way to contribute is to fork the CodeIgniter4 repository, and "clone" that to your development area. That sounds like some jargon, but "forking" on GitHub means "making a copy of that repo to your account" and "cloning" means "copying that code to your environment so you can work on it".
286+
The best way to contribute is to fork the CodeIgniter4 repository, and "clone"
287+
that to your development area. That sounds like some jargon, but "forking" on
288+
GitHub means "making a copy of that repo to your account" and "cloning" means
289+
"copying that code to your environment so you can work on it".
256290

257291
1. Set up Git ([Windows](https://git-scm.com/download/win), [Mac](https://git-scm.com/download/mac), & [Linux](https://git-scm.com/download/linux)).
258292
2. Go to the [CodeIgniter4 repository](https://github.com/codeigniter4/CodeIgniter4).
@@ -266,16 +300,21 @@ The best way to contribute is to fork the CodeIgniter4 repository, and "clone" t
266300
- If your PR is for bug fixes:
267301
- `> git switch develop`
268302
- `> git switch -c <new-branch-name>`
269-
- If your PR has any enhancement, create new branch from next minor version branch, e.g. __"4.3"__:
303+
- If your PR has any enhancement, create new branch from next minor version branch, e.g. __"4.5"__:
270304
- `> git switch <next-minor-version-branch>`
271305
- `> git switch -c <new-branch-name>`
272-
7. Fix existing bugs on the [Issue tracker](https://github.com/codeigniter4/CodeIgniter4/issues) after confirming that no one else is working on them.
306+
7. Fix existing bugs on the [Issue tracker](https://github.com/codeigniter4/CodeIgniter4/issues) after confirming that no one
307+
else is working on them.
273308
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.
274309
- `> git commit`
275-
- 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.
310+
- Commit messages are expected to be descriptive of why and what you changed
311+
specifically. Commit messages like "Fixes #1234" would be asked by the reviewer
312+
to be revised. [Atomic commit](https://en.wikipedia.org/wiki/Atomic_commit#Atomic_commit_convention) is recommended. See
313+
[Contribution Workflow](./workflow.md#commit-messages) for details.
276314
9. If you have touched PHP code, run static analysis.
277315
- `> composer analyze`
278-
10. Run unit tests on the specific file you modified. If there are no existing tests yet, please create one.
316+
10. Run unit tests on the specific file you modified. If there are no existing
317+
tests yet, please create one.
279318
- `> vendor/bin/phpunit tests/system/path/to/file/you/modified`
280319
- Make sure the tests pass to have a higher chance of merging.
281320
11. [Push](https://docs.github.com/en/github/using-git/pushing-commits-to-a-remote-repository) your contribution branch to your fork.
@@ -285,9 +324,13 @@ The best way to contribute is to fork the CodeIgniter4 repository, and "clone" t
285324

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

288-
The codebase maintainers will now be alerted to the submission and someone from the team will respond. If your change fails to meet the guidelines, it will be rejected or feedback will be provided to help you improve it.
327+
The codebase maintainers will now be alerted to the submission and someone from
328+
the team will respond. If your change fails to meet the guidelines, it will be
329+
rejected or feedback will be provided to help you improve it.
289330

290-
Once the maintainer handling your pull request is satisfied with it, they will approve the pull request and merge it into the "develop" branch. Your patch will now be part of the next release!
331+
Once the maintainer handling your pull request is satisfied with it, they will
332+
approve the pull request and merge it into the "develop" branch. Your patch will
333+
now be part of the next release!
291334

292335
## Translating System Messages
293336

contributing/styleguide.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
This document declares a set of coding conventions and rules to be followed when contributing PHP code
44
to the CodeIgniter project.
55

6-
**Note:**
6+
> [!NOTE]
77
> While we would recommend it, there's no requirement that you follow these conventions and rules in your
88
own projects. Usage is discretionary within your projects but strictly enforceable within the framework.
99

@@ -200,7 +200,7 @@ conditions MUST always be at the beginning or at the end of the line, not a mix
200200
- The conditional operator, also known simply as the ternary operator, MUST be preceded and followed by at least one space around both the `?` and `:` characters.
201201
- When the middle operand of the conditional operator is omitted, the operator MUST follow the same style rules as other binary comparison operators.
202202

203-
**Note:**
203+
> [!NOTE]
204204
> All the preceding rules are quoted from PSR-12. You may visit its website to view the code block samples.
205205
206206
## Custom Conventions

0 commit comments

Comments
 (0)