From f28e123ecf50b42961fabb883b5016d4cd42ae28 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 15 Jan 2013 08:35:25 +0100 Subject: [PATCH 1/4] tweaked the PR check-list to make it more readable --- contributing/code/patches.rst | 44 +++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/contributing/code/patches.rst b/contributing/code/patches.rst index 6080497fa7b..311cbf96859 100644 --- a/contributing/code/patches.rst +++ b/contributing/code/patches.rst @@ -265,7 +265,13 @@ pull request message, like in: .. tip:: Please use the title with "[WIP]" if the submission is not yet completed - or the tests are incomplete or not yet passing. + or the tests are incomplete or not yet passing. Also add a todo-list in + the pull-request description describing what still need to be done: + + .. code-block:: text + + - [ ] tests need to be updated + - [ ] documentation PR yet to be submitted The pull request description must include the following check list to ensure that contributions may be reviewed without needless feedback loops and that @@ -273,27 +279,31 @@ your contributions can be included into Symfony2 as quickly as possible: .. code-block:: text - Bug fix: [yes|no] - Feature addition: [yes|no] - Backwards compatibility break: [yes|no] - Deprecations: [what, when|no] - Symfony2 tests pass: [yes|no] - Fixes the following tickets: [comma separated list of tickets fixed by the PR] - Todo: [list of todos pending] - License of the code: MIT - Documentation PR: [The reference to the documentation PR if any] + | Q | A + | ------------- | --- + | Bug fix? | [yes|no] + | New feature? | [yes|no] + | BC breaks? | [yes|no] + | Deprecations? | [yes|no] + | Tests pass? | [yes|no] + | Fixed tickets | [comma separated list of tickets fixed by the PR] + | License | MIT + | Doc PR | [The reference to the documentation PR if any] An example submission could now look as follows: .. code-block:: text - Bug fix: no - Feature addition: yes - Backwards compatibility break: no - Fixes the following tickets: #12, #43 - Todo: - - License of the code: MIT - Documentation PR: symfony/symfony-docs#123 + | Q | A + | ------------- | --- + | Bug fix? | no + | New feature? | no + | BC breaks? | no + | Deprecations? | no + | Tests pass? | yes + | Fixed tickets | #12, #43 + | License | MIT + | Doc PR | symfony/symfony-docs#123 In the pull request description, give as much details as possible about your changes (don't hesitate to give code examples to illustrate your points). If From 4f4c0c34072605d6966f4f6c2b41825e3ad750aa Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 15 Jan 2013 08:40:47 +0100 Subject: [PATCH 2/4] added a shorter version of the todo list for minor changes --- contributing/code/patches.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/contributing/code/patches.rst b/contributing/code/patches.rst index 311cbf96859..ec5a8327a8e 100644 --- a/contributing/code/patches.rst +++ b/contributing/code/patches.rst @@ -305,6 +305,16 @@ An example submission could now look as follows: | License | MIT | Doc PR | symfony/symfony-docs#123 +For typos, minor changes in the PHPDocs, or changes in translation files, use +the shorter version of the check-list: + +.. code-block:: text + + | Q | A + | ------------- | --- + | Fixed tickets | [comma separated list of tickets fixed by the PR] + | License | MIT + In the pull request description, give as much details as possible about your changes (don't hesitate to give code examples to illustrate your points). If your pull request is about adding a new feature or modifying an existing one, From 16664689642d5391855e2964068afe990edfe7e6 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 15 Jan 2013 08:44:21 +0100 Subject: [PATCH 3/4] added more information about how to document BC breaks and deprecations --- contributing/code/patches.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contributing/code/patches.rst b/contributing/code/patches.rst index ec5a8327a8e..697c1a88166 100644 --- a/contributing/code/patches.rst +++ b/contributing/code/patches.rst @@ -200,10 +200,12 @@ Prepare your Patch for Submission When your patch is not about a bug fix (when you add a new feature or change an existing one for instance), it must also include the following: -* An explanation of the changes in the relevant CHANGELOG file(s); +* An explanation of the changes in the relevant CHANGELOG file(s) (the ``[BC + BREAK]`` or the ``[DEPRECATION]`` prefix must be used when relevant); * An explanation on how to upgrade an existing application in the relevant - UPGRADE file(s) if the changes break backward compatibility. + UPGRADE file(s) if the changes break backward compatibility or if you + deprecate something that will ultimately break backward compatibility. Step 3: Submit your Patch ------------------------- From ea65196ef166ab5d37d8d058c1e8544a6342701a Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Tue, 15 Jan 2013 08:59:06 +0100 Subject: [PATCH 4/4] added more hints about the goals of the check-list --- contributing/code/patches.rst | 51 +++++++++++++++++++++++++++-------- 1 file changed, 40 insertions(+), 11 deletions(-) diff --git a/contributing/code/patches.rst b/contributing/code/patches.rst index 697c1a88166..95970c3ae1e 100644 --- a/contributing/code/patches.rst +++ b/contributing/code/patches.rst @@ -264,17 +264,6 @@ pull request message, like in: [Yaml] fixed something [Form] [Validator] [FrameworkBundle] added something -.. tip:: - - Please use the title with "[WIP]" if the submission is not yet completed - or the tests are incomplete or not yet passing. Also add a todo-list in - the pull-request description describing what still need to be done: - - .. code-block:: text - - - [ ] tests need to be updated - - [ ] documentation PR yet to be submitted - The pull request description must include the following check list to ensure that contributions may be reviewed without needless feedback loops and that your contributions can be included into Symfony2 as quickly as possible: @@ -317,6 +306,46 @@ the shorter version of the check-list: | Fixed tickets | [comma separated list of tickets fixed by the PR] | License | MIT +Some answers to the questions trigger some more requirements: + + * If you answer yes to "Bug fix?", check if the bug is already listed in the + Symfony issues and reference it/them in "Fixed tickets"; + + * If you answer yes to "New feature?", you must submit a pull request to the + documentation and reference it under the "Doc PR" section; + + * If you answer yes to "BC breaks?", the patch must contain updates to the + relevant CHANGELOG and UPGRADE files; + + * If you answer yes to "Deprecations?", the patch must contain updates to the + relevant CHANGELOG and UPGRADE files; + + * If you answer no to "Tests pass", you must add an item to a todo-list with + the actions that must be done to fix the tests; + + * If the "license" is not MIT, just don't submit the pull request as it won't + be accepted anyway. + +If some of the previous requirements are not met, create a todo-list and add +relevant items: + +.. code-block:: text + + - [ ] fix the tests as they have not been updated yet + - [ ] submit changes to the documentation + - [ ] document the BC breaks + +If the code is not finished yet because you don't have time to finish it or +because you want early feedback on your work, add an item to todo-list: + +.. code-block:: text + + - [ ] finish the code + - [ ] gather feedback my changes + +As long as you have items in the todo-list, please prefix the pull request +title with "[WIP]". + In the pull request description, give as much details as possible about your changes (don't hesitate to give code examples to illustrate your points). If your pull request is about adding a new feature or modifying an existing one,