Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds feature where if review requirement not met, then request #30653

Merged
merged 17 commits into from
Jun 13, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion projects/github-actions/required-review/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ async function main() {
core.endGroup();

let matchedPaths = [];
const teamsNeededForReview = new Set()
const teamsNeededForReview = new Set();
for ( let i = 0; i < requirements.length; i++ ) {
const r = requirements[ i ];
core.startGroup( `Checking requirement "${ r.name }"...` );
Expand Down
20 changes: 9 additions & 11 deletions projects/github-actions/required-review/src/request-review.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
const core = require( '@actions/core' );
const github = require( '@actions/github' );
const getUsername = require( './get-username.js' );

/**
* Request review from the given team
*
* @param {string} team - GitHub team slug, or @ followed by a GitHub user name.
* @param {object} teams - GitHub team slug, or @ followed by a GitHub user name.
gfog-floqast marked this conversation as resolved.
Show resolved Hide resolved
*/
async function requestReviewer( teams ) {
const octokit = github.getOctokit( core.getInput( 'token', { required: true } ) );
const owner = github.context.payload.repository.owner.login;
const repo = github.context.payload.repository.name;
const pr = github.context.payload.pull_request.number;

let userReviews = []
let teamReviews = []
const userReviews = [];
const teamReviews = [];

for (var i = 0; i < teams.length; i++) {
const t = teams[ i ]
for ( const t of teams ) {
if ( t.startsWith( '@' ) ) {
userReviews.push( t.slice( 1 ) )
userReviews.push( t.slice( 1 ) );
} else {
teamReviews.push( t )
teamReviews.push( t );
}
}

Expand All @@ -31,11 +29,11 @@ async function requestReviewer( teams ) {
repo: repo,
pull_number: pr,
reviewers: userReviews,
team_reviewers: teamReviews
} )
team_reviewers: teamReviews,
} );
core.info( `Requested review(s) from ${ teams }` );
} catch ( err ) {
throw new Error( `Unable to request review.\n Error: ${err}` );
throw new Error( `Unable to request review.\n Error: ${ err }` );
}
}

Expand Down
49 changes: 25 additions & 24 deletions projects/github-actions/required-review/src/requirement.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class RequirementError extends SError {}
*/
function printSet( label, teamReviewers, neededTeams ) {
core.info( label + ' ' + ( teamReviewers.length ? teamReviewers.join( ', ' ) : '<empty set>' ) );
return {teamReviewers, neededTeams};
return { teamReviewers, neededTeams };
}

/**
Expand All @@ -32,13 +32,9 @@ function buildReviewerFilter( config, teamConfig, indent ) {
const team = teamConfig;
return async function ( reviewers ) {
const members = await fetchTeamMembers( team );
teamReviewers = reviewers.filter( reviewer => members.includes( reviewer ) );
const neededTeams = teamReviewers.length ? [] : [team];
printSet(
`${ indent }Members of ${ team }:`,
teamReviewers
);
return {teamReviewers, neededTeams};
const teamReviewers = reviewers.filter( reviewer => members.includes( reviewer ) );
const neededTeams = teamReviewers.length ? [] : [ team ];
return printSet( `${ indent }Members of ${ team }:`, teamReviewers, neededTeams );
};
}

Expand Down Expand Up @@ -91,15 +87,20 @@ function buildReviewerFilter( config, teamConfig, indent ) {
for ( const requirementResult of reviewersAny ) {
if ( requirementResult.teamReviewers.length != 0 ) {
requirementsMet.push( requirementResult.teamReviewers );
};
}
if ( requirementResult.neededTeams.length != 0 ) {
neededTeams.push( requirementResult.neededTeams );
};
};
if( requirementsMet.some( a => a.length > 0 ) ){ // If there are requirements met, zero out the needed teams
}
}
if ( requirementsMet.length > 0 ) {
// If there are requirements met, zero out the needed teams
neededTeams.length = 0;
};
return printSet( `${ indent }=>`, [ ...new Set( requirementsMet.flat( 1 ) )], [ ...new Set( neededTeams.flat( 1 ) ) ]);
}
return printSet(
`${ indent }=>`,
[ ...new Set( requirementsMet.flat( 1 ) ) ],
[ ...new Set( neededTeams.flat( 1 ) ) ]
);
};
}

Expand All @@ -110,17 +111,18 @@ function buildReviewerFilter( config, teamConfig, indent ) {
const requirementsMet = [];
const neededTeams = [];
for ( const requirementResult of reviewersAll ) {
if ( requirementResult.teamReviewers.length != 0 ) {
if ( requirementResult.teamReviewers.length !== 0 ) {
requirementsMet.push( requirementResult.teamReviewers );
};
if ( requirementResult.neededTeams.length != 0 ) {
}
if ( requirementResult.neededTeams.length !== 0 ) {
neededTeams.push( requirementResult.neededTeams );
};
};
if ( neededTeams.length !== 0 ) { // If there are needed teams, zero out requirements met
}
}
if ( neededTeams.length !== 0 ) {
// If there are needed teams, zero out requirements met
return printSet( `${ indent }=>`, [], [ ...new Set( neededTeams.flat( 1 ) ) ] );
};
return printSet( `${ indent }=>`, [ ...new Set( requirementsMet.flat( 1 ) ) ], [ ...new Set( neededTeams.flat( 1 ) ) ] );
}
return printSet( `${ indent }=>`, [ ...new Set( requirementsMet.flat( 1 ) ) ], [] );
};
}

Expand Down Expand Up @@ -241,10 +243,9 @@ class Requirement {
*/
async needsReviewsFrom( reviewers ) {
core.info( 'Checking reviewers...' );
const checkNeededTeams = await this.reviewerFilter( reviewers )
const checkNeededTeams = await this.reviewerFilter( reviewers );
return checkNeededTeams.neededTeams;
}
}

module.exports = Requirement;