Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ As a general rule, all Pull Requests undergo the QA verification, except for the

* Version branch merges
* Changes that have no functional impact (e.g. code comments), cannot be manually verified, or are verified by automated tests.
* Highly technical changes or changes without user-facing behavior, which may be validated by the maintainer team.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree with @jolelievre and @kpodemski : the QA Team should be included in the decision if the PR is too much technical and therefore is a "QA by dev".
Otherwise a maintainer could create pull request, approve it and test it by another maintainer to finally merge it without QA consulted before.
Example : PrestaShop/PrestaShop#40400

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cnavarro-prestashop

Otherwise a maintainer could create pull request, approve it and test it by another maintainer to finally merge it without QA consulted before.

Yeah, exactly. And you would be loosing exclusive control of the project, that the company doesn't want of course. :-)


The Pull Request can proceed to merge once the QA verification is successful.

Expand Down Expand Up @@ -232,7 +233,9 @@ If it has the right number of approvals (Pull Requests submitted to the [PrestaS

You can add `Waiting for QA` label to request a validation from QA team. Some PRs do not need QA testing, for example fixing a typo or a code change that only impacts CI.

Some other PRs may require QA by a developer, for example if the PR is very technical and does not change the behavior of the software. In that case, you can add `Waiting for QA` and `Waiting for dev` label. This should be discussed with QA functional team.
Some other PRs may require QA by a developer, for example if the PR is very technical and does not change the behavior of the software. In that case, the maintainer team may validate the change.

The decision to validate a Pull Request through developer QA is driven by the maintainer team, based on the technical nature and testability of the change. The QA team may be consulted when clarification is needed. Appropriate labels such as `Waiting for QA` and `Waiting for dev` can be applied accordingly.

If the PR behavior is confirmed by QA Team (or a developer, for the "Waiting for dev PRs), the PR can be merged. See above section `Merging Pull Requests` for the different actions needed following the merge.

Expand Down
4 changes: 2 additions & 2 deletions content/maintainers-guide/summary-github-lifecycle.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,15 @@ When the Pull Request has been approved (it needs two approvals on the Core repo

On regular Pull Requests, the QA team is in charge of testing the Pull Request. They will use the "How to test" part of the Pull Request description to validate the behavior implemented, and also run some more tests to validate there are no regressions.

Some Pull Requests however cannot be tested by QA team, the Developers team might validate them.
Some Pull Requests cannot be tested by the QA team, typically because they are highly technical in nature or do not involve user-facing behavior. In such cases, the maintainer team may validate them. The decision on who performs the validation is made by the QA team or the maintainers.

PrestaShop project has UI tests. These tests can be executed on each Pull Request thanks to [`ga.tests.ui.pr` tool](https://github.com/PrestaShop/ga.tests.ui.pr/). Please read the README file of this tool and fill the Pull Request description with the execution URL.

If the Pull Request is tested successfully, the label "QA approved" is applied. Else, the author is notified about the Issues found by the tests.

### Merging the Pull Request

Pull Requests that have been validated by QA can be merged. They must also be milestoned, and if they fix an issue, the issue must be labelled, milestoned, and closed.
Pull Requests that have been validated and tested can be merged. They must also be milestoned, and if they fix an issue, the issue must be labelled, milestoned, and closed.

[how-pull-requests]: {{< devdocs "contribute/contribution-process/how-pull-requests-are-processed/" >}}
[code-review]: ({{< relref "reviewing-pull-requests" >}})