QA should not be assigned to the PRs which were declined?

[skapral]

The precedent was first seen here.

IMO the fact that declined task was assigned on QA contradicts with paragraph 30 from policy, which in current version states that:

“Quality Review.” If you have QA role in the project, you will be asked to review jobs after their completion.

The PR caused questions from QA. However, what is the reason to question the PR which was already declined because of bad quality on earlier phases?

[0crat]

@yegor256/z please, pay attention to this issue

[yegor256]

@skapral imagine a situation: there is a PR, which was reviewed by REV and rejected. We have to pay to REV and ARC. We need to confirm that they actually did their job right. We need QA to validate that. See?

[skapral]

@yegor256 good argument, didn't thought of that from this perspective.

So, taking your response literally, am I got it correctly that QA is supposed to estimate the quality of DEV/REV/ARC work, not the quality of final artifacts provided? Even if so, there is some ambiguity in procedure.

Imagine the situation: some PR was rejected because of bad quality and lost DEV, who was too lazy and procrastinated to fix the remarks in time. REV did his work and made this rubbish rejected. And here comes QA, sees that the PR is rubbish, literally follows the procedure and puts "bad quality" verdict. What happens next?

Is REV supposed to be rewarded? According to the common sense - he should be rewarded, his part is done, he rejected the rubbish. According to your comment, taken literally, he should not. By putting "quality is bad" QA states that DEV and ARC did their job wrong.

Again, what QA is estimating: participants efforts quality or final artifacts quality? IMO there is no reason to estimate the artifacts one more time - REV and ARC supposed to did it already. And if it is about estimating participants and their effords, the checklist from here IMO worth deep reconsideration.

What do you think? If you agree, I will proceed with the alternate proposal - this comment is too long for it.

[yegor256]

@skapral QA pays attention to the quality of process, not the quality of code/artifacts. QA will validate the work of the REV and ARC. If they managed to reject the PR correctly, the quality will be "acceptable". Let's ask them to confirm that this is how they understand our current Policy. @ypshenychka @elenavolokhova what do you think?

[skapral]

@yegor256

If they managed to reject the PR correctly, the quality will be "acceptable".

Policy doesn't state it anywhere. It has paragraph 30, which just enlists the possible outcomes, and paragraph 42, which enlists a list of requirements to good and bad outcomes. IMO the least which should be done is to make it explicit that "if REV or ARC rejected the PR with quality violations, verdict of QA must be acceptable, unless" some unforgivable violations are made.

[elenavolokhova]

@yegor256 Yes, this is how I understand our Policy. But I agree with @skapral that it is not perfectly explained and some logic lives in our minds instead of Policy.
I mean:

• QA evaluates the quality of process, not the quality of code/artifacts.
• DEV is responsible and paid for Issue and should ask issue reporter to fix all process issues found (like missing description, formatting or addressing). Otherwise DEVs work is not completed in terms of process.
• REV is responsible and paid for PR and should ask PR author to fix all process issues found (like missing description, formatting or addressing). Otherwise REVs work is not completed in terms of process.

Also, we should add definition of each quality mark, as mentioned above. Something like:

• "Good": there are no violations of Policy rules
• "Acceptable": there are no unforgivable violations of Policy rules, issues are fixed and responsible person provided confirmation to QA remarks if any
• "Bad": there are unforgivable violations of Policy rules

I think it would make our process more transparent to everyone.
Please, correct me, if I'm wrong.

[skapral]

@elenavolokhova, some corrections

DEV is responsible and paid for Issue and should ask issue reporter to fix all process issues found (like missing description, formatting or addressing). Otherwise DEVs work is not completed in terms of process.
REV is responsible and paid for PR and should ask PR author to fix all process issues found (like missing description, formatting or addressing). Otherwise REVs work is not completed in terms of process.

I wouldn't put it that straightforward yet. For example, IMO the best candidate for monitoring fixing the issue description and title is ARC - he is the one who puts the issue in scope and he has an ability to reject the issue on early stage and spend project resources from it.

I think it may be a subject for a good separate issue.

"Acceptable": there are no unforgivable violations of Policy rules, issues are fixed and responsible person provided confirmation to QA remarks if any.

Issue may not be fixed yet, when the PR is rejected. Also, there is a bit of ambiguity with confirmations to QA when the work is rejected, see issue #317.

[ypshenychka]

@yegor256 You're right. This is how I understand the Policy too. It's difficult to document all possible cases and QA reaction on them, but the most common situation may be written with more detailed description of when we consider ticket as 'good\acceptable\bad' to avoid future discussions.