Skip to content

Commit 20f2ff3

Browse files
committed
Merge branch '2.7' into 2.8
* 2.7: [symfony#5480] minor tweaks Added instructions about testing a PR Integrated PR comments Wording Fixed numbering Fixed case Improved sample comment Added link to PRs in need of review Added initial draft of "Community Reviews" page
2 parents 84e2495 + 5837c18 commit 20f2ff3

File tree

3 files changed

+210
-0
lines changed

3 files changed

+210
-0
lines changed

Diff for: contributing/code/patches.rst

+2
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,8 @@ Check that all tests still pass and push your branch remotely:
253253
254254
$ git push --force origin BRANCH_NAME
255255
256+
.. _contributing-code-pull-request:
257+
256258
Make a Pull Request
257259
~~~~~~~~~~~~~~~~~~~
258260

Diff for: contributing/community/index.rst

+1
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,5 @@ Community
55
:maxdepth: 2
66

77
releases
8+
reviews
89
other

Diff for: contributing/community/reviews.rst

+207
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
Community Reviews
2+
=================
3+
4+
Symfony is an open-source project driven by a large community. If you don't feel
5+
ready to contribute code or patches, reviewing issues and pull requests (PRs)
6+
can be a great start to get involved and give back. In fact, people who "triage"
7+
issues are the backbone to Symfony's success!
8+
9+
Why Reviewing Is Important
10+
--------------------------
11+
12+
Community reviews are essential for the development of the Symfony framework,
13+
since there are many more pull requests and bug reports than there are members
14+
in the Symfony core team to review, fix and merge them.
15+
16+
On the `Symfony issue tracker`_, you can find many items in a `Needs Review`_
17+
status:
18+
19+
* **Bug Reports**: Bug reports need to be checked for completeness.
20+
Is any important information missing? Can the bug be *easily* reproduced?
21+
22+
* **Pull Requests**: Pull requests contain code that fixes a bug or implements
23+
new functionality. Reviews of pull requests ensure that they are implemented
24+
properly, are covered by test cases, don't introduce new bugs and maintain
25+
backwards compatibility.
26+
27+
Note that **anyone who has some basic familiarity with Symfony and PHP can
28+
review bug reports and pull requests**. You don't need to be an expert to help.
29+
30+
Be Constructive
31+
---------------
32+
33+
Before you begin, remember that you are looking at the result of someone else's
34+
hard work. A good review comment thanks the contributor for their work,
35+
identifies what was done well, identifies what should be improved and suggests a
36+
next step.
37+
38+
Create a GitHub Account
39+
-----------------------
40+
41+
Symfony uses GitHub_ to manage bug reports and pull requests. If you want to
42+
do reviews, you need to `create a GitHub account`_ and log in.
43+
44+
The Bug Report Review Process
45+
-----------------------------
46+
47+
A good way to get started with reviewing is to pick a bug report from the
48+
`bug reports in need of review`_.
49+
50+
The steps for the review are:
51+
52+
#. **Is the Report Complete?**
53+
54+
Good bug reports contain a link to a fork of the `Symfony Standard Edition`_
55+
(the "reproduction project") that reproduces the bug. If it doesn't, the
56+
report should at least contain enough information and code samples to
57+
reproduce the bug.
58+
59+
#. **Reproduce the Bug**
60+
61+
Download the reproduction project and test whether the bug can be reproduced
62+
on your system. If the reporter did not provide a reproduction project,
63+
create one by forking_ the `Symfony Standard Edition`_. Reproduce the bug
64+
with the instructions given by the reporter.
65+
66+
#. **Update the Issue Status**
67+
68+
At last, add a comment to the bug report. **Thank the reporter for reporting
69+
the bug**. Include the line ``Status: <status>`` in your comment to update
70+
the status of the ticket. This line will trigger our `Carson Bot`_ which
71+
updates the labels of the issue accordingly. You can set the status to one of
72+
the following:
73+
74+
**Needs Work** If the bug does not contain enough information to be
75+
reproduced, explain what information is missing and move the report to this
76+
status.
77+
78+
**Works for me** If the bug *does* contain enough information to be
79+
reproduced but works on your system, or if the reported bug is a feature and
80+
not a bug, provide a short explanation and move the report to this status.
81+
82+
**Reviewed** If you can reproduce the bug, move the report to this status.
83+
If you created a reproduction project, include the link to the project in
84+
your comment.
85+
86+
.. topic:: Example
87+
88+
Here is a sample comment for a bug report that could be reproduced:
89+
90+
.. code-block:: text
91+
92+
Thank you @weaverryan for creating this bug report! This indeed looks
93+
like a bug. I reproduced the bug in the "kernel-bug" branch of
94+
https://github.com/webmozart/symfony-standard.
95+
96+
Status: Reviewed
97+
98+
The Pull Request Review Process
99+
-------------------------------
100+
101+
The process for reviewing pull requests (PRs) is similar to doing a review of a
102+
bug report. Reviews of pull requests usually take a little longer since you need
103+
to understand the functionality that has been fixed or implemented and then find
104+
out whether the implementation is complete.
105+
106+
It is okay to do partial reviews. If you do a partial review, comment how far
107+
you got and leave the PR in "Needs Review" state.
108+
109+
Pick a pull request from the `PRs in need of review`_ and follow these steps:
110+
111+
#. **Is the PR Complete**?
112+
113+
Every pull request must contain a header that gives some basic information
114+
about the PR. You can find the template for that header in the
115+
:ref:`Contribution Guidelines <contributing-code-pull-request>`.
116+
117+
#. **Is the Base Branch Correct?**
118+
119+
GitHub displays the branch that a PR is based on below the title of the
120+
pull request. Is that branch correct?
121+
122+
* Bugs should be fixed in the oldest, maintained version that contains the
123+
bug. Check :doc:`Symfony's Release Schedule <releases>` to find the oldest
124+
currently supported version.
125+
126+
* New features should always be added to the current development version.
127+
Check the `Symfony Roadmap`_ to find the current development version.
128+
129+
#. **Reproduce the Problem**
130+
131+
Read the issue that the pull request is supposed to fix. Reproduce the
132+
problem on a clean `Symfony Standard Edition`_ project and try to understand
133+
why it exists.
134+
135+
#. **Review the Code**
136+
137+
Read the code of the pull request and check it against some common criteria:
138+
139+
* Does the code address the issue the PR is intended to fix/implement?
140+
* Does the PR stay within scope to address *only* that issue?
141+
* Does the PR contain automated tests? Do those tests cover all relevant
142+
edge cases?
143+
* Does the PR contain sufficient comments to easily understand its code?
144+
* Does the code break backwards compatibility? If yes, does the PR header say
145+
so?
146+
* Does the PR contain deprecations? If yes, does the PR header say so? Does
147+
the code contain ``trigger_error()`` statements for all deprecated
148+
features?
149+
* Are all deprecations and backwards compatibility breaks documented in the
150+
latest UPGRADE-X.X.md file? Do those explanations contain "Before"/"After"
151+
examples with clear upgrade instructions?
152+
153+
Eventually, some of these aspects will be checked automatically.
154+
155+
#. **Test the Code**
156+
157+
Take your project from step 3 and test whether the PR works properly.
158+
Replace the Symfony vendor by the code in the PR by running the following Git
159+
commands. Insert the PR ID for the ``<ID>`` placeholders:
160+
161+
.. code-block:: text
162+
163+
$ cd vendor/symfony/symfony
164+
$ git fetch origin pull/<ID>/head:pr<ID>
165+
$ git checkout pr<ID>
166+
167+
Now you can test the project against the code in the PR.
168+
169+
#. **Update the PR Status**
170+
171+
At last, add a comment to the PR. **Thank the contributor for working on the
172+
PR**. Include the line ``Status: <status>`` in your comment to update the
173+
status of the ticket. This line will trigger our `Carson Bot`_ which updates
174+
the labels of the issue accordingly. You can set the status to one of the
175+
following:
176+
177+
**Needs Work** If the PR is not yet ready to be merged, explain the issues
178+
that you found and move it to this status.
179+
180+
**Reviewed** If the PR satisfies all the checks above, move it to this
181+
status. A core contributor will soon look at the PR and decide whether it can
182+
be merged or needs further work.
183+
184+
.. topic:: Example
185+
186+
Here is a sample comment for a PR that is not yet ready for merge:
187+
188+
.. code-block:: text
189+
190+
Thank you @weaverryan for working on this! It seems that your test
191+
cases don't cover the cases when the counter is zero or smaller.
192+
Could you please add some tests for that?
193+
194+
Status: Needs Work
195+
196+
.. _GitHub: https://github.com
197+
.. _Symfony issue tracker: https://github.com/symfony/symfony/issues
198+
.. _Symfony Standard Edition: https://github.com/symfony/symfony-standard
199+
.. _create a GitHub account: https://help.github.com/articles/signing-up-for-a-new-github-account/
200+
.. _forking: https://help.github.com/articles/fork-a-repo/
201+
.. _bug reports in need of review: https://github.com/symfony/symfony/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+label%3A%22Bug%22+label%3A%22Status%3A+Needs+Review%22+
202+
.. _PRs in need of review: https://github.com/symfony/symfony/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+label%3A%22Status%3A+Needs+Review%22+
203+
.. _Contribution Guidelines: https://github.com/symfony/symfony/blob/master/CONTRIBUTING.md
204+
.. _Symfony's Release Schedule: http://symfony.com/doc/current/contributing/community/releases.html#schedule
205+
.. _Symfony Roadmap: https://symfony.com/roadmap
206+
.. _Carson Bot: https://github.com/carsonbot/carsonbot
207+
.. _`Needs Review`: https://github.com/symfony/symfony/labels/Status%3A%20Needs%20Review

0 commit comments

Comments
 (0)