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