Adapt developer QA guidelines

[Hlavtox]

Questions	Answers
Description	This PR clarifies how decisions are made on whether a change requires functional QA or can be validated by a maintainer. It removes ambiguity by explicitly stating that this is a technical decision handled by the maintainer team, which is best positioned to assess the nature and testability of the change. This helps keep Pull Requests moving efficiently, reflects the existing project workflow, and prevents recurring misunderstandings around QA validation.
Fixed ticket	N/A

cc @Robin-Fischer-PS @kpodemski

[kpodemski]

Requested changes will be consulted with the decision makers.

[jolelievre]

I disagree with this change, I fully agree that some PRs should be tested by devs and it's more efficient. Some modifications are also purely technical and don't change the functional behaviour so in this case QA by dev is relevant.

However, it's not up to the dev to decide that the QA functional is out of the loop, the QA team must be integrated in the decision at least to say "ok a dev can test it". Because any modification, even a refacto, could have an unexpected side effect so it seems hazardous to leave QA out of the process IMHO

[cnavarro-prestashop]

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

[Hlavtox]

@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. :-)

[Hlavtox]

@jolelievre @cnavarro-prestashop

Nobody is suggesting to exclude QA from the process entirely. The point of my proposal is speed up and unblock PRs that do not require manual functional QA and are currently stuck for weeks or months despite being purely technical.

We are investing quite a lot of money, my time, Dominik's time, into the project for years now and we constantly have to wait even months for QA, or even answer we can dev QA it.

Why cannot other investors have their own QA? Everything that we merge is properly developer tested, with screenshots, with UI tests run. We never test our own PRs. We know what to focus on, what changes can break.

Because any modification, even a refacto, could have an unexpected side effect so it seems hazardous to leave QA out of the process IMHO

Yes, it can, but a chance for a non-developer person to find it is low, in all respect.

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

If the concern is about potential abuse (for example, maintainers approving and testing their own changes without sufficient oversight), then I can assure you I am the last person who would allow some crap code to go into the core. You know very well that I am nitpicking even if you find your solution usable. 👍

[Robin-Fischer-PS]

Hi all,

I'm not going to repeat what was said earlier by @jolelievre or @cnavarro-prestashop , with which I generally agree :)

Just a few reactions and clarifications:

we constantly have to wait even months for QA, or even answer we can dev QA it.

I know this isn't a criticism of the QA team, but to answer that question: Yes, that's true, pings often get lost in the notifications. I try to check them regularly, but some slip through. For other QAs, it's harder to get their attention because they receive so many GitHub notifications every day that it becomes unmanageable (I'm lucky enough to now only receive the direct ping or qa-functional ping 😅 ).
We tried different solutions, none have worked well for now...

Yes, it can, but a chance for a non-developer person to find it is low, in all respect.

It depends of the technical profile of the QA. Some have almost no coding knowledge, some a little more. For example, I'm far from considering myself a developer (I once was, a long long time ago...), nevertheless I can read code, understand algorithms, etc... And thus having a general idea of where to look, what impacts a refacto (or other "without user-facing behavior" PR) can have.
We (= the QA) rarely test only the fix/modification added by a PR, but we also check for potential other impacts and side-effects, and that's where most bugs hides.

If the concern is about potential abuse (for example, maintainers approving and testing their own changes without sufficient oversight), then I can assure you I am the last person who would allow some crap code to go into the core.

To be perfectly honest about this, we discussed it this morning with Cyril and Jo, and the problem isn't so much what you and Dominik are doing, but the fact that it could "open the door" to other people who are less meticulous (whether intentionally or not).

Why cannot other investors have their own QA? Everything that we merge is properly developer tested, with screenshots, with UI tests run. We never test our own PRs.

Well, there was indeed a “QA council” at one time, which has somewhat fallen into disuse, whose purpose was to open up external contributions on QA topics as well (I do not recall if you applied at the time, to be honest).
Right now it's on hold. I don't know if this is something we could rework, if it would follow our current company guidelines etc...

And finally, to quote @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.

It's not just "without QA consulted before" but also "without anyone else consulted".
And without getting into the debate about exclusive control by PrestaShop SA you mentioned earlier @Hlavtox , it can still cause problems in general (if we don't have the same vision of what needs to be done for the project, etc... But it's a way too big subject for me right now 😅 ).

[Hlavtox]

Hello @Robin-Fischer-PS !

And thus having a general idea of where to look, what impacts a refacto (or other "without user-facing behavior" PR) can have.
We (= the QA) rarely test only the fix/modification added by a PR, but we also check for potential other impacts and side-effects, and that's where most bugs hides.

Yes, but I think that I am more than well qualified to do this, pointing out things that would other people miss. Example being - PrestaShop/hummingbird#892

To be perfectly honest about this, we discussed it this morning with Cyril and Jo, and the problem isn't so much what you and Dominik are doing, but the fact that it could "open the door" to other people who are less meticulous (whether intentionally or not).

About the same risk as letting almost anyone to be a commiter. They can as well do damage by merging things, with no QA. Everyone involved is expected to act responsibly, review code properly, and bear responsibility for their actions. If someone is careless, the problem is not who is allowed to contribute, but how reviews and accountability are handled.

It's not just "without QA consulted before" but also "without anyone else consulted".

Well, the PR has to be "consulted" with at least other two commiters, which is usually enough to know if the PR is correct. There is no need to consult anyone else. 👍