dev: Update the instructions for pull requests

The checklist must now be filled by the reviewer to ease the
contributions.

.github/pull_request_template.md

@@ -1,28 +1,32 @@
 ## Related issue(s)
 
-<!-- Copy-paste the URL to the related issue(s) if any ("N/A" if not applicable). -->
+<!-- Copy-paste the URL to the related issue(s) if any ("N/A" if not applicable).
+  -->
 
 ## How to test manually
 
 <!-- List actions (step by step) of what have to be done in order to test your
-  -- changes manually ("N/A" if not applicable). -->
+  -- changes manually ("N/A" if not applicable).
+  -->
 
 ## Checklist
 
-<!-- Make sure all the todos are checked before asking for review. If you think
-  -- one of the item isn’t applicable to this PR, please check it anyway and
-  -- precise "N/A" next to it.
-  -- Get help: https://github.com/Probesys/bileto/blob/main/CONTRIBUTING.md#open-a-pull-request-pr
+<!-- Please don't change or remove this checklist. It will be used by the
+  -- reviewer to make sure to not forget important things.
+  -- Reviewer: if you think one of the item isn’t applicable to this PR,
+  -- please check it anyway.
+  -- Get help: https://github.com/Probesys/bileto/blob/main/docs/developers/review.md
   -->
 
-- [ ] code is manually tested
-- [ ] permissions / authorizations are verified
-- [ ] data can be imported correctly
-- [ ] interface works on both mobiles and big screens
-- [ ] interface works in both light and dark modes
-- [ ] interface works on both Firefox and Chrome
-- [ ] accessibility has been tested
-- [ ] tests are up-to-date
-- [ ] locales are synchronized
-- [ ] copyright notices are up to date
-- [ ] documentation is up to date (including migration notes)
+- [ ] Code is manually tested
+- [ ] Permissions / authorizations are verified
+- [ ] New data can be imported
+- [ ] Interface works on both mobiles and big screens
+- [ ] Interface works in both light and dark modes
+- [ ] Interface works on both Firefox and Chrome
+- [ ] Accessibility has been tested
+- [ ] Translations are synchronized
+- [ ] Tests are up to date
+- [ ] Copyright notices are up to date
+- [ ] Documentation is up to date
+- [ ] Pull request has been reviewed and approved

CONTRIBUTING.md

@@ -62,97 +62,10 @@ Once you’re done with your changes:
 When you open your pull request, you’ll be asked some information:
 
 - a related ticket opened in the bugtracker (the one you’ve been assigned);
-- how we can test the changes manually (instructions step by step in order to verify that your changes work);
-- and a checklist to make sure that you (and we) don’t forget anything important.
+- how we can test the changes manually (instructions step by step in order to verify that your changes work).
 
-Some items of the checklist may feel out of context for your pull request: please check them anyway and add “N/A” at the end of the line.
-
-If you don’t know how to do a specific task of the checklist, please take a look at the following explanations.
-If something is still not clear, let we know so we can assist you.
-There’s no reasons to feel ashamed of not knowing something, and you’ll hopefully learn something on your way.
-
-#### “Code is manually tested”
-
-This is to remind you to test your changes yourself.
-The idea is to follow the instructions that you gave in the “How to test manually” section, and to verify that it works as expected.
-Sometimes, you’ll realize at this stage that you‘ve forgotten something important.
-
-#### “Permissions / authorizations are verified”
-
-Bileto is based on a custom system of roles.
-This system is explained in the document [“Roles & permissions”](/docs/developers/roles.md).
-This item reminds you to put the required actions behind an authorization.
-
-#### “Data can be imported correctly”
-
-When new entities or fields are created, we need to consider making this data importable by [the `DataImporter` service](/src/Service/DataImporter/DataImporter.php).
-However, not all data needs to be imported.
-If fields are changed or removed, the service needs to be adjusted as well.
-
-#### “Interface works on both mobile and big screen”
-
-We want Bileto to work on desktop and mobiles.
-If you’ve changed the interface, you should check that it still works on mobile.
-For that, you can use the “mobile mode” of your browser (e.g. <kbd>CTRL + ALT + M</kbd> in Firefox).
-
-#### “Interface works in both light and dark modes”
-
-Bileto provides light and dark modes to suit user preferences.
-If you have used the colors as explained in [the `colors.light.css` file](/assets/stylesheets/variables/colors.light.css), everything should already be fine.
-However, you should still manually check that it works in both modes.
-You can change the mode in the “Preferences” page of Bileto.
-
-#### “Interface works on both Firefox and Chrome”
-
-We aim to make Bileto work well on all major browsers.
-We strongly support Firefox and it
-This is our standard web browser and we develop with it.
-However, Chrome is the number one brower in terms of market share.
-Please always try your changes with at least Firefox and Chrome (or Chromium, or at least a Webkit browser).
-
-#### “Accessibility has been tested”
-
-We want Bileto to be as accessible as possible.
-An accessibility issue in Bileto is considered as a bug.
-However, testing the accessibility can be pretty hard when you’re not an expert.
-To get started, we recommend you to use the [WAVE browser extension](https://wave.webaim.org/extension/).
-This extension highlight some usual issues.
-Please try to fix the ones related to your changes.
-
-Additional testing can include testing with a screen reader, or directly with people who need accessibility features.
-However, these solutions are more difficult to get right.
-
-#### “Tests are up-to-date”
-
-Everything in Bileto is not tested, but we try to write the most pertinent tests.
-In this context, we mainly write [“Application Tests”](https://symfony.com/doc/current/testing.html#application-tests) to automate the tests in Bileto.
-If you’ve made a change at a controller level, it’s likely that we’ll ask you to write a test about your change.
-The tests are located under [the `tests/` folder](/tests) and are written with [PHPUnit](https://docs.phpunit.de).
-[Learn how to execute the tests.](/docs/developers/tests.md)
-
-#### “Locales are synchronized”
-
-Bileto is provided in English and in French.
-You can run the command `make translations` to verify that everything is translated.
-Please make sure to sort the translations in alphabetical order in the translations files.
-[Learn more about the translations.](/docs/developers/translations.md)
-
-#### “Copyright notices are up to date”
-
-At the top of each file, we include a small comment to indicate the copyright of the file and the license.
-If you’ve made a significant change to the file, please add a line:
-
-```
-Copyright <year> <your name>
-```
-
-#### “Documentation is up to date”
-
-Keeping the documentation up to date is a difficult task.
-We try to make it easy by making sure that we’ve documented everything relevant in each pull request.
-The documentation includes everything under [`docs/`](/docs), the [“readme”](/README.md), and the migration notes in the [changelog](/CHANGELOG.md).
-The migration notes are important information that needs to be carried out when upgrading to the next version of Bileto.
-It’s not often that we have to document this, though.
+The checklist is intended to the reviewers: you don't have to fill it.
+It allows us to make sure that you (and we) don’t forget anything important.
 
 ## Contribute to the documentation
 

docs/developers/README.md

@@ -29,6 +29,7 @@ Dedicated to the frontend:
 Dedicated to the maintainers:
 
 - [Managing the dependencies](/docs/developers/dependencies.md)
+- [Reviewing the pull requests](/docs/developers/review.md)
 - [How to release a version](/docs/developers/release.md)
 
 Architecture:

docs/developers/review.md

@@ -0,0 +1,98 @@
+# Reviewing the pull requests
+
+Each change to the repository must be reviewed through a code review.
+A template with a checklist is provided and filled for each pull request.
+
+Some items of the checklist may feel out of context: please check them anyway.
+
+If you don’t know how to do a specific task of the checklist, please take a look at the following explanations.
+
+## “Code is manually tested”
+
+This is to remind you to test the changes yourself.
+The idea is to follow the instructions given in the “How to test manually” section, and to verify that it works as expected.
+Sometimes, you’ll realize at this stage that something important has been forgotten.
+
+## “Permissions / authorizations are verified”
+
+Bileto is based on a custom system of roles.
+This system is explained in the document [“Roles & permissions”](/docs/developers/roles.md).
+In particular, controllers' actions must be protected by the appropriate authorization checks.
+
+## “New data can be imported correctly”
+
+When new entities or fields are created, we need to consider making this data importable by [the `DataImporter` service](/src/Service/DataImporter/DataImporter.php).
+However, not all data needs to be imported.
+If fields are changed or removed, the service needs to be adjusted as well.
+
+## “Interface works on both mobiles and big screens”
+
+Bileto must work on desktop and mobiles.
+If the interface is changed, you should check that it still works on mobiles.
+For that, you can use the “mobile mode” of your browser (e.g. <kbd>CTRL + ALT + M</kbd> in Firefox).
+
+## “Interface works in both light and dark modes”
+
+Bileto provides light and dark modes to suit user preferences.
+If colors are properly used as explained in [the `colors.light.css` file](/assets/stylesheets/variables/colors.light.css), everything should already be fine.
+However, you should still manually check that it works in both modes.
+You can change the mode in the “Preferences” page of Bileto.
+
+## “Interface works on both Firefox and Chrome”
+
+Bileto must work well on all major browsers.
+We strongly support Firefox and it is our standard web browser as we develop with it.
+However, Chrome is the number one brower in terms of market share.
+Please always try your changes with at least Firefox and Chrome (or Chromium, or at least a Webkit browser).
+
+## “Accessibility has been tested”
+
+An inaccessible application is a broken one and an accessibility issue must be considered as a bug.
+However, testing the accessibility can be pretty hard when you’re not an expert.
+To get started, you can use the [WAVE browser extension](https://wave.webaim.org/extension/).
+This extension highlight some usual issues.
+Please try to fix the ones related to the changes of the pull/merge request.
+
+Additional testing can include testing with a screen reader, or directly with people who need accessibility features.
+However, these solutions are more difficult to get right.
+
+## “Translations are synchronized”
+
+Bileto is provided in English and in French.
+The French translation must be synchronized with the English one.
+
+You can run the command `make translations` to verify that everything is translated.
+Please make sure to sort the translations in alphabetical order in the translations files.
+
+[Learn more about the translations.](/docs/developers/translations.md)
+
+## “Tests are up to date”
+
+Everything in Bileto is not tested, but we try to write the most pertinent tests.
+In this context, we mainly write [“Application Tests”](https://symfony.com/doc/current/testing.html#application-tests) to automate the tests in Bileto.
+If a change impacts a controller, it’s likely that some tests must be written about the change.
+The tests are located under [the `tests/` folder](/tests) and are written with [PHPUnit](https://docs.phpunit.de).
+
+[Learn how to execute the tests.](/docs/developers/tests.md)
+
+## “Copyright notices are up to date”
+
+At the top of each file, we include a small comment to indicate the copyright of the file and the license.
+Developer must add a line if they made a significant change to the file:
+
+```
+Copyright <year> <name>
+```
+
+## “Documentation is up to date”
+
+Keeping the documentation up to date is a difficult task.
+We try to make it easy by making sure that we’ve documented everything relevant in each pull request.
+The documentation includes everything under [`docs/`](/docs), the [“readme”](/README.md), and the migration notes in the [changelog](/CHANGELOG.md).
+
+The migration notes are important information that needs to be carried out when upgrading to the next version of Bileto.
+It’s not often that we have to document this, though.
+
+## “Merge request has been reviewed and approved”
+
+It is to remind you to review the changes in the code and to approve the pull request.