mirror of
https://github.com/HDFGroup/hdf5.git
synced 2026-09-25 04:09:44 +03:00
review-checklist: exempt a just-requested reviewer from avalanche pruning (#6524)
A direct review_requested (e.g. manually re-requesting a reviewer via the GitHub UI) adds a second currently-requested owner to that reviewer's area — indistinguishable from an unpruned CODEOWNERS avalanche, so the avalanche-detection pass immediately removed them again on the same run. Carve out the login this run's review_requested action names before running avalanche detection.
This commit is contained in:
@@ -450,6 +450,16 @@ async function coordinateReviewers(github, context, core, {
|
||||
}
|
||||
}
|
||||
|
||||
// The specific login a direct review_requested action just added, if any —
|
||||
// see its use below in avalanche detection. A deliberate re-request must
|
||||
// survive this same run: it lands existingRequested at two owners for that
|
||||
// login's area (them plus whoever an earlier pruning pass already picked),
|
||||
// which is indistinguishable from an unpruned CODEOWNERS avalanche unless
|
||||
// this login is carved out.
|
||||
const justRequestedLogin = (action === 'review_requested' && context.payload.requested_reviewer)
|
||||
? context.payload.requested_reviewer.login
|
||||
: null;
|
||||
|
||||
const existingRequested = new Set(
|
||||
prData.requested_reviewers.map(r => r.login).filter(Boolean).filter(l => !updatedExcluded.has(l))
|
||||
);
|
||||
@@ -561,8 +571,12 @@ async function coordinateReviewers(github, context, core, {
|
||||
// synchronize-swap or additive-fill logic runs, so those paths see an already-
|
||||
// correct one-per-area baseline. (PR #6484: user pushed a commit that first
|
||||
// touched .github; GitHub assigned all 4 .github CODEOWNERS simultaneously.)
|
||||
// justRequestedLogin's area is exempted — a human just deliberately asked
|
||||
// for exactly that person, which looks identical to an avalanche (two
|
||||
// owners now requested) but isn't one.
|
||||
const avalancheAreas = eligibleAreas.filter(
|
||||
area => area.owners.filter(o => existingRequested.has(o)).length > 1
|
||||
&& !(justRequestedLogin && area.owners.includes(justRequestedLogin))
|
||||
);
|
||||
if (avalancheAreas.length > 0) {
|
||||
const { selected: avalanchePruned, log: pruneLog } = chooseReviewers(avalancheAreas, {
|
||||
|
||||
@@ -898,6 +898,46 @@ asyncTest('coordinateReviewers: synchronize with one owner per area does not pru
|
||||
assert.strictEqual(github.calls.removeRequestedReviewers.length, 0);
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------
|
||||
// coordinateReviewers — a direct review_requested must survive avalanche
|
||||
// detection on the very same run (reported bug: manually re-requesting a
|
||||
// reviewer via the GitHub UI got them immediately removed again, because
|
||||
// their area now had two currently-requested owners — themselves plus
|
||||
// whichever pick an earlier ready_for_review pruning pass had already made
|
||||
// — which is indistinguishable from an unpruned CODEOWNERS avalanche unless
|
||||
// the just-requested login is carved out).
|
||||
// ----------------------------------------------------------------
|
||||
|
||||
asyncTest('coordinateReviewers: review_requested for a specific login is not undone by avalanche pruning', async () => {
|
||||
const github = makeGithubMock();
|
||||
const context = {
|
||||
eventName: 'pull_request_target',
|
||||
payload: {
|
||||
action: 'review_requested',
|
||||
requested_reviewer: { login: 'jhendersonHDF' },
|
||||
sender: { type: 'User' },
|
||||
},
|
||||
};
|
||||
const args = makeCoordinateBaseArgs({
|
||||
prData: {
|
||||
user: { login: 'lrknox' },
|
||||
draft: false,
|
||||
// hyoklee is the load-balanced pick an earlier pass already made;
|
||||
// jhendersonHDF was just manually re-requested on top of it.
|
||||
requested_reviewers: [{ login: 'hyoklee' }, { login: 'jhendersonHDF' }],
|
||||
},
|
||||
// Rig the load-balancer so a fresh pick would land on hyoklee, not
|
||||
// jhendersonHDF — proving jhendersonHDF survives because they were
|
||||
// just requested, not because they'd have won the pick anyway.
|
||||
reviewerLoad: { hyoklee: 0, jhendersonHDF: 99, glennsong09: 0 },
|
||||
});
|
||||
|
||||
const { confirmedRequested } = await coordinateReviewers(github, context, makeCore(), args);
|
||||
|
||||
assert.strictEqual(github.calls.removeRequestedReviewers.length, 0, 'Nothing should be removed');
|
||||
assert.ok(confirmedRequested.has('jhendersonHDF'), 'The just-requested reviewer must stay');
|
||||
});
|
||||
|
||||
// ----------------------------------------------------------------
|
||||
// coordinateReviewers — bot-self-triggered review_request_removed must not
|
||||
// create a sticky exclusion (the bot's own removeUnselected/removeRequestedReviewers
|
||||
|
||||
Reference in New Issue
Block a user