Skip to content

Commit 6afcadc

Browse files
committed
Update the contributing docs
1 parent 074bc78 commit 6afcadc

5 files changed

Lines changed: 91 additions & 64 deletions

File tree

contributing/README.rst

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ If you've found a critical vulnerability, we'd be happy to credit you in our
5454
Tips for a Good Issue Report
5555
****************************
5656

57-
Use a descriptive subject line (eg parser library chokes on commas) rather than a vague one (eg. your code broke).
57+
Use a descriptive subject line (eg parser library chokes on commas) rather than
58+
a vague one (eg. your code broke).
5859

5960
Address a single issue in a report.
6061

@@ -64,12 +65,13 @@ Explain what you expected to happen, and what did happen.
6465
Include error messages and stacktrace, if any.
6566

6667
Include short code segments if they help to explain.
67-
Use a pastebin or dropbox facility to include longer segments of code or screenshots - do not include them in the issue report itself.
68+
Use a pastebin or dropbox facility to include longer segments of code or
69+
screenshots - do not include them in the issue report itself.
6870
This means setting a reasonable expiry for those, until the issue is resolved or closed.
6971

7072
If you know how to fix the issue, you can do so in your own fork & branch, and submit a pull request.
7173
The issue report information above should be part of that.
7274

7375
If your issue report can describe the steps to reproduce the problem, that is great.
74-
If you can include a unit test that reproduces the problem, that is even better, as it gives whoever is fixing
75-
it a clearer target!
76+
If you can include a unit test that reproduces the problem, that is even better,
77+
as it gives whoever is fixing it a clearer target!

contributing/guidelines.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,12 @@ Change Log
6464

6565
The change-log, in the user guide root, needs to be kept up-to-date.
6666
Not all changes will need an entry in it, but new classes, major or BC changes
67-
to existing classes, and bug fixes should.
67+
to existing classes should. Once we have a stable release, bug fixes would
68+
appear in the changelog too.
6869

69-
See the `CodeIgniter 4 change log
70-
<https://github.com/codeigniter4/CodeIgniter/blob/develop/user_guide_src/source/changelog.rst>`_
71-
for an example.
70+
The changelog is independently maintained by the framework release manager
71+
Make sure that your PR descriptions help us decide if the contribution should
72+
be highlighted in the next release after it has been merged.
7273

7374
PHP Compatibility
7475
=================
@@ -89,7 +90,7 @@ with earlier versions of the framework.
8990
Mergeability
9091
============
9192

92-
Your PRs need to be mergeable before they will be considered.
93+
Your PRs need to be mergeable and GPG-signed before they will be considered.
9394

9495
We suggest that you synchronize your repository's ``develop`` branch with
9596
that in the main repository, and then your feature branch and

contributing/internals.rst

Lines changed: 62 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,19 @@
22
CodeIgniter Internals Overview
33
##############################
44

5-
This guide should help contributors understand how the core of the framework works, and what needs to be done
6-
when creating new functionality. Specifically, it details the information needed to create new packages for the
7-
core.
5+
This guide should help contributors understand how the core of the framework works,
6+
and what needs to be done when creating new functionality. Specifically, it
7+
details the information needed to create new packages for the core.
88

99
Dependencies
1010
============
1111

12-
All packages should be designed to be completely isolated from the rest of the packages. This will allow
13-
them to be used in projects outside of CodeIgniter. Basically, this means that all dependencies should be
14-
kept to a minimum. Any dependencies must be able to be passed into the constructor. If you do need to use one
15-
of the other core packages, you can create that in the constructor using the Services class, as long as you
16-
provide a way for dependencies to override that::
12+
All packages should be designed to be completely isolated from the rest of the
13+
packages, if possible. This will allow them to be used in projects outside of CodeIgniter.
14+
Basically, this means that any dependencies should be kept to a minimum.
15+
Any dependencies must be able to be passed into the constructor. If you do need to use one
16+
of the other core packages, you can create that in the constructor using the
17+
Services class, as long as you provide a way for dependencies to override that::
1718

1819
public function __construct(Foo $foo=null)
1920
{
@@ -26,91 +27,112 @@ Type hinting
2627
============
2728

2829
PHP7 provides the ability to `type hint <http://php.net/manual/en/functions.arguments.php#functions.arguments.type-declaration>`_
29-
method parameters and return types. Use it where possible. Return type hinting is not always practical, but do try to
30-
make it work.
30+
method parameters and return types. Use it where possible. Return type hinting
31+
is not always practical, but do try to make it work.
3132

3233
At this time, we are not using strict type hinting.
3334

3435
Abstractions
3536
============
3637

37-
The amount of abstraction required to implement a solution should be the minimal amount required. Every layer of
38-
abstraction brings additional levels of technical debt and unnecessary complexity. That said, don't be afraid to
39-
use it when it's needed and can help things.
38+
The amount of abstraction required to implement a solution should be the minimal
39+
amount required. Every layer of abstraction brings additional levels of technical
40+
debt and unnecessary complexity. That said, don't be afraid to use it when it's
41+
needed and can help things.
4042

4143
* Don't create a new container class when an array will do just fine.
4244
* Start simple, refactor as necessary to achieve clean separation of code, but don't overdo it.
4345

4446
Testing
4547
=======
4648

47-
Any new packages submitted to the framework must be accompanied by unit tests. The target is 80%+ coverage of all
48-
classes within the package.
49+
Any new packages submitted to the framework must be accompanied by unit tests.
50+
The target is 80%+ code coverage of all classes within the package.
4951

5052
* Test only public methods, not protected and private unless the method really needs it due to complexity.
5153
* Don't just test that the method works, but test for all fail states, thrown exceptions, and other pathways through your code.
5254

55+
You should be aware of the extra assertions that we have made, provisions for
56+
accessing private properties for testing, and mock services.
57+
We have also made a **CITestStreamFilter** to capture test output.
58+
Do check out similar tests in ``tests/system/``, and read the "Testing" section
59+
in the user guide, before you dig in to your own.
60+
61+
Some testing needs to be done in a separate process, in order to setup the
62+
PHP globals to mimic test situations properly. See
63+
``tests/system/HTTP/ResponseSendTest`` for an example of this.
64+
5365
Namespaces and Files
5466
====================
5567

56-
All new packages should live under the ``CodeIgniter`` namespace. The package itself will need its own sub-namespace
68+
All new packages should live under the ``CodeIgniter`` namespace.
69+
The package itself will need its own sub-namespace
5770
that collects all related files into one grouping, like ``CodeIgniter\HTTP``.
5871

59-
Files MUST be named the same as the class they hold, and they must match the :doc:`Style Guide <styleguide>`, meaning
60-
CamelCase class and file names. The should be in their own directory that matches the sub-namespace under the **system**
61-
directory.
72+
Files MUST be named the same as the class they hold, and they must match the
73+
:doc:`Style Guide <./styleguide.rst>`, meaning CamelCase class and file names.
74+
They should be in their own directory that matches the sub-namespace under the
75+
**system** directory.
6276

63-
The the Router as an example. The Router lives in the ``CodeIgniter\Router`` namespace. It has two classes,
64-
**RouteCollection** and **Router**, which are in the files, **system/Router/RouteCollection.php** and
77+
Take the Router class as an example. The Router lives in the ``CodeIgniter\Router``
78+
namespace. Its two main classes,
79+
**RouteCollection** and **Router**, are in the files **system/Router/RouteCollection.php** and
6580
**system/Router/Router.php** respectively.
6681

6782
Interfaces
6883
----------
6984

70-
Most base classes should have an interface defined for them. At the very least this allows them to be easily mocked
71-
and passed in other classes as a dependency without breaking the type-hinting. The interface names should match
72-
the name of the class with "Interface" appended to it, like ``RouteCollectionInterface``.
85+
Most base classes should have an interface defined for them.
86+
At the very least this allows them to be easily mocked
87+
and passed to other classes as a dependency, without breaking the type-hinting.
88+
The interface names should match the name of the class with "Interface" appended
89+
to it, like ``RouteCollectionInterface``.
7390

7491
The Router package mentioned above includes the
75-
``CodeIgniter\Router\RouterCollectionInterface`` and ``CodeIgniter\Router\RouterInterface``
92+
``CodeIgniter\Router\RouteCollectionInterface`` and ``CodeIgniter\Router\RouterInterface``
7693
interfaces to provide the abstractions for the two classes in the package.
7794

7895
Handlers
7996
--------
8097

81-
When a package supports multiple "drivers", the convention is to place them in a **Handlers** directory, and
82-
name the child classes as Handlers. You will often find that creating a ``BaseHandler`` the child classes can
83-
extend to be beneficial in keeping the code DRY.
98+
When a package supports multiple "drivers", the convention is to place them in
99+
a **Handlers** directory, and name the child classes as Handlers.
100+
You will often find that creating a ``BaseHandler``, that the child classes can
101+
extend, to be beneficial in keeping the code DRY.
84102

85103
See the Log and Session packages for examples.
86104

87105
Configuration
88106
=============
89107

90-
Should the package require user-configurable settings, you should create a new file just for that package under
91-
**app/Config**. The file name should generally match the package name.
108+
Should the package require user-configurable settings, you should create a new
109+
file just for that package under **app/Config**.
110+
The file name should generally match the package name.
92111

93112
Autoloader
94113
==========
95114

96-
All files within the package should be added to **system/Config/AutoloadConfig.php**, in the "classmap" property.
97-
This is only used for core framework files, and helps to minimize file system scans and keep performance high.
115+
All files within the package should be added to **system/Config/AutoloadConfig.php**,
116+
in the "classmap" property. This is only used for core framework files, and helps
117+
to minimize file system scans and keep performance high.
98118

99119
Command-Line Support
100120
====================
101121

102-
CodeIgniter has never been known for it's strong CLI support. However, if your package could benefit from it, create a
103-
new file under **system/Commands**. The class contained within is simply a controller that is intended for CLI
104-
usage only. The ``index()`` method should provide a list of available commands provided by that package.
122+
CodeIgniter has never been known for it's strong CLI support. However, if your
123+
package could benefit from it, create a new file under **system/Commands**.
124+
The class contained within is simply a controller that is intended for CLI
125+
usage only. The ``index()`` method should provide a list of available commands
126+
provided by that package.
105127

106-
Routes must be added to **system/Config/Routes.php** using the ``cli()`` method to ensure it is not accessible
107-
through the browser, but is restricted to the CLI only.
128+
Routes must be added to **system/Config/Routes.php** using the ``cli()`` method
129+
to ensure it is not accessible through the browser, but is restricted to the CLI only.
108130

109131
See the **MigrationsCommand** file for an example.
110132

111133
Documentation
112134
=============
113135

114-
All packages must contain appropriate documentation that matches the tone and style of the rest of the user guide.
115-
In most cases, the top portion of the package's page should be treated in tutorial fashion, while the second
116-
half would be a class reference.
136+
All packages must contain appropriate documentation that matches the tone and
137+
style of the rest of the user guide. In most cases, the top portion of the package's
138+
page should be treated in tutorial fashion, while the second half would be a class reference.

contributing/signing.rst

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,22 @@ The developer pushing a commit as part of a PR isn't necessarily the person
99
who committed it originally, if the commit is not signed. This distorts the
1010
commit history and makes it hard to tell where code came from.
1111

12-
If a person "signs" a commit, they are free to use any name, specifically
12+
If a person "signs off" a commit, they are free to use any name, specifically
1313
one not their own. Again, the commit history cannot be relied on to determine
1414
the origin of the code, if one developer is spoofing another. A malicious person
1515
could commit bad code (for instance a virus) and make it look like another
1616
developer created it.
1717

1818
The best solution, while not fool-proof, is to "securely sign" your
19-
commits. Such commits are digitally signed, with a GPG-key, and
19+
commits. Such commits are digitally signed, with a GPG-key
2020
associated with your github account. It still isn't foolproof, because
2121
a malicious developer could create a bogus email and account, but it is
22-
more reliable than an unsigned or a "signed" commit.
22+
more reliable than an unsigned or a "signed-off by" commit.
2323

2424
If you don't sign your commits, we **may** accept your contribution,
2525
assuming it meets usefulness and contribution guidelines, but only
2626
if it isn't critical code and only after checking it carefully.
27-
If code performs an important role, we will insist that it be signed, and if
28-
it is critical code (however we interpret that), we will insist that your
29-
contributions be securely signed.
27+
If code performs an important role, we will insist that it be securely signed.
3028

3129
Read below to find out how to sign your commits :)
3230

contributing/workflow.rst

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,14 @@ which requires all pull requests to be sent to the "develop" branch. This is
2525
where the next planned version will be developed. The "master" branch will
2626
always contain the latest stable version and is kept clean so a "hotfix" (e.g:
2727
an emergency security patch) can be applied to master to create a new version,
28-
without worrying about other features holding it up. For this reason all
28+
without worrying about other features holding it up. For this reason, all
2929
commits need to be made to "develop" and any sent to "master" will be closed
3030
automatically. If you have multiple changes to submit, please place each
3131
change into their own branch on your fork.
3232

3333
One thing at a time: a pull request should only contain one change. That does
3434
not mean only one commit, but one change - however many commits it took. The
35-
reason for this is that if you change X and Y but send a pull request for both
35+
reason for this is that if you change X and Y but send a single pull request for both
3636
at the same time, we might really want X but disagree with Y, meaning we
3737
cannot merge the request. Using the Git-Flow branching model you can create
3838
new branches for both of these features and send two requests.
@@ -45,7 +45,8 @@ in your github account. You can make changes to your forked repository, while
4545
you cannot do the same with the shared one - you have to submit pull requests
4646
to it instead.
4747

48-
`Creating a fork <https://help.github.com/articles/fork-a-repo/>`_ is done through the Github website. Navigate to `our
48+
`Creating a fork <https://help.github.com/articles/fork-a-repo/>`_
49+
is done through the Github website. Navigate to `our
4950
repository <https://github.com/codeigniter4/CodeIgniter4>`_,
5051
click the **Fork** button in the top-right of the page, and choose which account or
5152
organization of yours should contain that fork.
@@ -70,7 +71,7 @@ Synching
7071

7172
Within your local repository, Git will have created an alias, **origin**, for the
7273
Github repository it is bound to. You want to create an alias for the shared
73-
repository, so that you can "synch" the two, making sure that your repository
74+
repository as well, so that you can "synch" the two, making sure that your repository
7475
includes any other contributions that have been merged by us into the shared repo::
7576

7677
git remote add upstream UPSTREAM_URL
@@ -98,8 +99,11 @@ Branching Revisited
9899
The top of this page talked about the **master** and **develop** branches.
99100
The *best practice* for your work is to create a *feature branch* locally,
100101
to hold a group of related changes (source, unit testing, documentation,
101-
change log, etc). This local branch should be named appropriately,
102-
for instance "fix/problem123" or "new/mind-reader".
102+
change log, etc).
103+
104+
This local branch should be named appropriately, for instance
105+
"fix/problem123" or "new/mind-reader". The slashes in these branch names is
106+
optional, and implies a sort of namespacing if used.
103107

104108
For instance, make sure you are in the *develop* branch, and create a
105109
new feature branch, based on *develop*, for a new feature you are creating::
@@ -113,7 +117,7 @@ Committing
113117
==========
114118

115119
Your local changes need to be *committed* to save them in your local repository.
116-
This is where `contribution signing <signing>`_ comes in.
120+
This is where `contribution signing <./signing.rst>`_ comes in.
117121

118122
You can have as many commits in a branch as you need to "get it right".
119123
For instance, to commit your work from a debugging session::
@@ -179,13 +183,13 @@ Make sure that the PR title is helpful for the maintainers and other developers.
179183
Add any comments appropriate, for instance asking for review.
180184

181185
.. note::
182-
If you do not provide a title for your PR, the odds of it being summarily rejected
186+
If you do not provide a title or description for your PR, the odds of it being summarily rejected
183187
rise astronomically.
184188

185189
When your PR is submitted, a continuous integration task will be triggered,
186190
running all the unit tests as well as any other checking we have configured for it.
187191
If the unit tests fail, or if there are merge conflicts, your PR will not
188-
be mergeable until fixed.
192+
be mergeable until those are fixed.
189193

190194
Fix such changes locally, commit them properly, and then push your branch again.
191195
That will update the PR automatically, and re-run the CI tests. You don't need

0 commit comments

Comments
 (0)