Make relatedTicket mandatory in the PR's description #94
Conversation
atomiix
commented
Jun 30, 2020
atomiix
commented
| Questions | Answers |
|---|---|
| Description? | This PR aims to make the "Fixed ticket" field of the PR's description mandatory |
| Type? | new feature |
| BC breaks? | no |
| Deprecations? | no |
| Fixed ticket? | Partially fixes PrestaShop/prestashop-retro#67 |
| How to test? | Tests must pass |
There was a problem hiding this comment.
Hi, I am very sorry but I am against this PR as it is.
I have checked the last 10 PRs submitted, see all of these examples, from both PrestaShop team and the community, they are all relevant and have no ticket linked:
- Functional tests - Separate browser page and page objects PrestaShop#19997
- Update license headers for PS 1.7.6.6 PrestaShop#19984
- Product/Category: allow specifying minimal quantity for products search PrestaShop#19976
- Rename AddCustomizationFieldsCommand to AddCustomizationCommand PrestaShop#19955
- Deprecate constants that are no longer used PrestaShop#19944
- Update Composer dependencies and prestashop module versions PrestaShop#19943
- Adjust UpdatePack command to be able to remove items PrestaShop#19938
Requiring a ticket for all of these issues is irrelevant and will just make the contribution process heavier and more annoying. Not everything has to be written in a ticket. If I upgrade symfony from v3.4.27 to v3.4.28 should I create a ticket for it ?
You see what I mean 😉 I advocate here for some pragmatism.
We can improve this PR by finding a better message. Something like "a ticket is very important, if your PR fixes an issue it must be reported" or "it is possible to send a PR without a related ticket but it might not be considered a priority".
The idea is to highlight the benefits and needs of a ticket while allowing relevant PRs to avoid needing one 😉
| @@ -142,11 +142,13 @@ public function isARefacto() | |||
| } | |||
|
|
|||
| /** | |||
| * @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or create a new issue so that it can be reviewed and tested.") | |||
There was a problem hiding this comment.
| * @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or create a new issue so that it can be reviewed and tested.") | |
| * @Assert\NotBlank(message = "Each pull request should have a ticket linked. Please link it to an existing issue or [create a new issue](https://github.com/PrestaShop/PrestaShop/issues/new/choose) so that it can be reviewed and tested.") |
|
Hi @atomiix, the message has been reworded, cf. PrestaShop/prestashop-retro#67 (comment). |
|
@matks, I don't have the right to request your review so here is a little ping to ask for your review, thanks! |
Here we go 😊 it's ✅ |
