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

required-review: Add is-author-or-reviewer condition #41966

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from
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
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions projects/github-actions/required-review/.gitattributes
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
/.gitattributes production-exclude
/.gitignore production-exclude
/test.sh production-exclude
/tests/** production-exclude
8 changes: 4 additions & 4 deletions projects/github-actions/required-review/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ The requirements consist of an array of requirement objects. A requirement objec
* `teams` is an array of strings that are GitHub team slugs in the organization or repository. A
review is required from a member of any of these teams.

Instead of a string, a single-keyed object may be specified. The key is either `all-of` or
`any-of`, and the value is an array as for `teams`. When the key is `all-of`, a review is required
from every team (but if a person is a member of multiple teams, they can satisfy multiple
requirements). When it's `any-of`, one review from any team is needed.
Instead of a string, a single-keyed object may be specified. The key is `all-of`, `any-of`, or `is-author-or-reviewer`, and the value is an array as for `teams`.
When the key is `all-of`, a review is required from every team (but if a person is a member of multiple teams, they can satisfy multiple requirements).
When it's `any-of`, one review from any team is needed.
When it's `is-author-or-reviewer`, it works like `any-of` but the author of the PR is considered to have self-reviewed.

Additionally, you can specify a single user by prefixing their username with `@`. For example,
`@example` will be treated as a virtual team with one member; `example`.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: added

Add `is-author-or-reviewer` condition.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Fix check for empty team lists.
4 changes: 3 additions & 1 deletion projects/github-actions/required-review/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
"scripts": {
"build-development": [
"pnpm run build"
]
],
"test-coverage": "pnpm run test-coverage",
"test-js": "pnpm run test"
},
"repositories": [
{
Expand Down
7 changes: 5 additions & 2 deletions projects/github-actions/required-review/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,12 @@
"picomatch": "4.0.2"
},
"devDependencies": {
"@vercel/ncc": "0.36.1"
"@vercel/ncc": "0.36.1",
"jest": "29.7.0"
},
"scripts": {
"build": "ncc build src/main.js -o dist --source-map --license licenses.txt"
"build": "ncc build src/main.js -o dist --source-map --license licenses.txt",
"test": "jest --config=tests/jest.config.js --verbose",
"test-coverage": "pnpm run test --coverage"
}
}
44 changes: 35 additions & 9 deletions projects/github-actions/required-review/src/requirement.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const assert = require( 'assert' );
const core = require( '@actions/core' );
const github = require( '@actions/github' );
const { SError } = require( 'error' );
const picomatch = require( 'picomatch' );
const fetchTeamMembers = require( './team-members.js' );
Expand Down Expand Up @@ -52,32 +53,29 @@ function buildReviewerFilter( config, teamConfig, indent ) {
const op = keys[ 0 ];
let arg = teamConfig[ op ];

// Shared validation.
switch ( op ) {
case 'any-of':
case 'all-of':
case 'is-author-or-reviewer':
// These ops require an array of teams/objects.
if ( ! Array.isArray( arg ) ) {
throw new RequirementError( `Expected an array of teams, got ${ typeof arg }`, {
config: config,
value: arg,
} );
}
if ( ! arg.length === 0 ) {
if ( arg.length === 0 ) {
throw new RequirementError( 'Expected a non-empty array of teams', {
config: config,
value: teamConfig,
} );
}
arg = arg.map( t => buildReviewerFilter( config, t, `${ indent } ` ) );
break;

default:
throw new RequirementError( `Unrecognized operation "${ op }"`, {
config: config,
value: teamConfig,
} );
}

// Process operations.
if ( op === 'any-of' ) {
return async function ( reviewers ) {
core.info( `${ indent }Union of these:` );
Expand Down Expand Up @@ -126,7 +124,35 @@ function buildReviewerFilter( config, teamConfig, indent ) {
};
}

// WTF?
if ( op === 'is-author-or-reviewer' ) {
return async function ( reviewers ) {
core.info( `${ indent }Author or reviewers are union of these:` );
const authorOrReviewers = [ ...reviewers, github.context.payload.pull_request.user.login ];
const reviewersAny = await Promise.all(
arg.map( f => f( authorOrReviewers, `${ indent } ` ) )
);
const requirementsMet = [];
const neededTeams = [];
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.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 ) ) ]
);
};
}

throw new RequirementError( `Unrecognized operation "${ op }"`, {
config: config,
value: teamConfig,
Expand Down Expand Up @@ -239,7 +265,7 @@ class Requirement {
* Test whether this requirement is satisfied.
*
* @param {string[]} reviewers - Reviewers to test against.
* @return {boolean} Whether the requirement is satisfied.
* @return {string[]} Array of teams from which review is still needed.
*/
async needsReviewsFrom( reviewers ) {
core.info( 'Checking reviewers...' );
Expand Down
10 changes: 10 additions & 0 deletions projects/github-actions/required-review/tests/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
const path = require( 'path' );
const coverageConfig = require( 'jetpack-js-tools/jest/config.coverage.js' );

module.exports = {
...coverageConfig,
rootDir: path.resolve( __dirname, '..' ),
roots: [ '<rootDir>/src/', '<rootDir>/tests/' ],
resolver: require.resolve( 'jetpack-js-tools/jest/jest-resolver.js' ),
setupFilesAfterEnv: [ require.resolve( 'jetpack-js-tools/jest/setup-console.js' ) ],
};
Loading
Loading