-
Notifications
You must be signed in to change notification settings - Fork 4
Add independent peer review validation workflow #64
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
Draft
AndrewGable
wants to merge
65
commits into
main
Choose a base branch
from
andrew-validate-independent-peer-review
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 62 commits
Commits
Show all changes
65 commits
Select commit
Hold shift + click to select a range
731d00c
Add independent peer review validation workflow
AndrewGable aba7b19
Use GitHub App client ID for peer review check
AndrewGable 2d2a6c9
Use GraphQL for peer review validation
AndrewGable 89bebf1
Simplify peer review validation helpers
AndrewGable e72fba8
Add newline
AndrewGable f473b3b
Hard-code OS Botify app ID
AndrewGable 23b2a2c
use client-id
AndrewGable 61d554c
Fix OS Botify token permissions
AndrewGable 0b48d74
Rename peer review workflow
AndrewGable e57f967
Rename peer review verifier
AndrewGable b6aea85
Clarify peer review failures
AndrewGable 2d8ce9c
Test informational peer review checks
AndrewGable 4bee49d
Undo draft
AndrewGable 1d3a559
Show informational peer review failures
AndrewGable 74415cd
Restore hard peer review failure
AndrewGable 74efe1d
Pass peer review check before approval
AndrewGable 72a99fd
Use Melvin app for peer review check
AndrewGable fec8f31
Pin Node 24 and upgrade setup-node to v6 with npm cache
roryabraham 76b600f
Add GithubUtils wrapper with rate limiting
roryabraham 9eafdd8
Extract peer review logic into testable modules
roryabraham da27d6e
Normalize co-author email parsing to match Web-Expensify chore
roryabraham 104bf8e
Extract evaluatePeerReview pass/skip/fail decision function
roryabraham c690b0c
Fail peer review check when branch protection cannot be read
roryabraham a886588
Add unit tests for peer review verification
roryabraham 0a8b0f8
Use Peer Review Checker bot and fix workflow triggers
roryabraham 11c368d
Add test and typecheck CI workflows and local dev docs
roryabraham 82015d8
Remove OSBotify guard from test and typecheck jobs
roryabraham 0ba1286
Rename GithubUtils to GitHubAPI and standardize module exports
roryabraham c61abf0
Rename GitHubAPI class to GitHubAPIClient
roryabraham a20f5bd
Switch peer review verification to CLI-based arguments
roryabraham 6a0d11f
Use published expensify-common 2.0.184 for CLI parsing
roryabraham 7a72ee9
Add ESLint with eslint-config-expensify and lint CI workflow
roryabraham f817ca1
Upgrade actions/create-github-app-token to 3.2.0
roryabraham 3e08588
app-id -> client-id
roryabraham f4bdf04
Alphabetize scripts
roryabraham cf5b09d
refactor: move libs to scripts/libs
roryabraham 4236743
Add Prettier with CI check
roryabraham fa4e331
refactor(peer-review): drop eventContext eslint overrides
roryabraham d082b36
refactor(tests): drop test eslint overrides
roryabraham 41b95cb
refactor: extract CLI wrapper to scripts/libs/CLI.ts
roryabraham 334a6d6
chore: upgrade expensify-common to 2.0.188 for CLI subpath
roryabraham 1537ef7
style: fix prettier formatting in eventContext.ts
roryabraham b3d954a
Remove unhelpful docs
roryabraham c3b65ea
ci: broaden workflow path filters for JS/TS extensions
roryabraham 3b1ce0d
ci: remove redundant CI=true from workflow env
roryabraham af679b5
refactor: flatten peer review libs into reusable modules
roryabraham e243313
refactor: rename coAuthorEmails to parseCoAuthorEmails
roryabraham 2296821
Run prettier
roryabraham 87af15a
refactor: extract uniqueSorted into CollectionUtils
roryabraham 38a4caf
refactor: split GitHubUtils into focused submodules
roryabraham 4a6aa55
feat: detect GitHub App bots via isBotUser helper
roryabraham 3630fe4
refactor: rename CLI arg to --pull-request-number
roryabraham e38935d
refactor: inline CLI parsing in verifyPeerReview main
roryabraham 40831be
refactor: inline peer review failure handler in main catch
roryabraham a944774
refactor: extract prSlug in evaluatePeerReview
roryabraham ec927c2
feat: resolve co-authors from display name fallback
roryabraham 62b825a
Fix prettier
roryabraham 817d866
fix: throw when canonical commit author cannot be resolved
roryabraham 7c496a8
Fix prettier
roryabraham 3a3b3e8
fix: attribute authors from referenced PR on merge commits
roryabraham de60ef3
Revert "fix: attribute authors from referenced PR on merge commits"
roryabraham f496ec3
Merge origin/main into andrew-validate-independent-peer-review
roryabraham 18facec
Merge origin/main into andrew-validate-independent-peer-review
roryabraham d7c2da9
Add OSBotify and CLABotify to known bot users
roryabraham 6f10b59
Match employee approvers case-insensitively
roryabraham File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| name: Lint | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - "**/*.[tj]s" | ||
| - "**/*.?[tj]s" | ||
| - "eslint.config.mjs" | ||
| - "tsconfig.json" | ||
| - "package.json" | ||
| - "package-lock.json" | ||
| - ".nvmrc" | ||
| - ".github/workflows/lint.yml" | ||
|
|
||
| jobs: | ||
| lint: | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| - name: Checkout | ||
| # 6.0.3 | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 | ||
|
|
||
| # v6.1.0 | ||
| - name: Setup Node | ||
| uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f | ||
| with: | ||
| node-version-file: .nvmrc | ||
| cache: npm | ||
| cache-dependency-path: package-lock.json | ||
|
|
||
| - name: Install npm packages | ||
| run: npm ci | ||
|
|
||
| - name: Lint | ||
| run: npm run lint | ||
|
|
||
| - name: Tell people how to run the failing check | ||
| if: failure() | ||
| run: echo "::error::The lint check failed! To run it locally, run \`npm ci && npm run lint\` from the GitHub-Actions repo root." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| name: Prettier | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - "**/*.[tj]s" | ||
| - "**/*.?[tj]s" | ||
| - ".prettierrc.mjs" | ||
| - ".prettierignore" | ||
| - "package.json" | ||
| - "package-lock.json" | ||
| - ".nvmrc" | ||
| - ".github/workflows/prettier.yml" | ||
|
|
||
| jobs: | ||
| prettier: | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| - name: Checkout | ||
| # 6.0.3 | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 | ||
|
|
||
| # v6.1.0 | ||
| - name: Setup Node | ||
| uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f | ||
| with: | ||
| node-version-file: .nvmrc | ||
| cache: npm | ||
| cache-dependency-path: package-lock.json | ||
|
|
||
| - name: Install npm packages | ||
| run: npm ci | ||
|
|
||
| - name: Verify there's no Prettier diff | ||
| run: | | ||
| npm run prettier -- --log-level error | ||
| if ! git diff --name-only --exit-code; then | ||
| echo "::error::Prettier diff detected! Run \`npm run prettier\` from the GitHub-Actions repo root and commit the changes." | ||
| exit 1 | ||
| fi |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| name: Test | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - "**/*.[tj]s" | ||
| - "**/*.?[tj]s" | ||
| - "tsconfig.json" | ||
| - "package.json" | ||
| - "package-lock.json" | ||
| - ".nvmrc" | ||
| - ".github/workflows/test.yml" | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| - name: Checkout | ||
| # 6.0.3 | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 | ||
|
|
||
| # v6.1.0 | ||
| - name: Setup Node | ||
| uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f | ||
| with: | ||
| node-version-file: .nvmrc | ||
| cache: npm | ||
| cache-dependency-path: package-lock.json | ||
|
|
||
| - name: Install npm packages | ||
| run: npm ci | ||
|
|
||
| - name: Run unit tests | ||
| run: npm test | ||
|
|
||
| - name: Tell people how to run the failing check | ||
| if: failure() | ||
| run: echo "::error::The test check failed! To run it locally, run \`npm ci && npm test\` from the GitHub-Actions repo root." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| name: Typecheck | ||
|
|
||
| on: | ||
| pull_request: | ||
| types: [opened, synchronize] | ||
| paths: | ||
| - "**/*.ts" | ||
| - "**/*.[mc]ts" | ||
| - "tsconfig.json" | ||
| - "package.json" | ||
| - "package-lock.json" | ||
| - ".nvmrc" | ||
| - ".github/workflows/typecheck.yml" | ||
|
|
||
| jobs: | ||
| typecheck: | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| - name: Checkout | ||
| # 6.0.3 | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 | ||
|
|
||
| # v6.1.0 | ||
| - name: Setup Node | ||
| uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f | ||
| with: | ||
| node-version-file: .nvmrc | ||
| cache: npm | ||
| cache-dependency-path: package-lock.json | ||
|
|
||
| - name: Install npm packages | ||
| run: npm ci | ||
|
|
||
| - name: Typecheck | ||
| run: npm run typecheck | ||
|
|
||
| - name: Tell people how to run the failing check | ||
| if: failure() | ||
| run: echo "::error::The typecheck check failed! To run it locally, run \`npm ci && npm run typecheck\` from the GitHub-Actions repo root." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| # Note: This workflow is configured to run on all pull requests throughout the Expensify org, not just this repo. | ||
| # That has a few consequences: | ||
| # - We need to checkout the repo it's running on, and not just the GitHub-Actions repo | ||
| # - branch and path matching does not work in the workflow layer. From the docs: https://docs.github.com/en/enterprise-cloud@latest/repositories/configuring-branches-and-merges-in-your-repository/managing-rulesets/available-rules-for-rulesets#supported-event-triggers | ||
| # > Any filters you specify for the supported events are ignored - for example, branches, branches-ignore, paths, types and so on. The workflow is only triggered, and is always triggered, by the default activity types of the supported events | ||
| name: Verify peer review | ||
| run-name: Verify peer review for ${{ github.repository }}#${{ github.event.pull_request.number }} | ||
|
|
||
| on: | ||
| pull_request: | ||
| pull_request_review: | ||
| types: [submitted, dismissed] | ||
|
|
||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
|
|
||
| jobs: | ||
| verifyPeerReview: | ||
| name: Check independent approval | ||
| runs-on: blacksmith-2vcpu-ubuntu-2404 | ||
| steps: | ||
| # v3.2.0 | ||
| - name: Generate a GitHub App token | ||
| id: generateAppToken | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 | ||
| with: | ||
| client-id: "3877737" | ||
| private-key: ${{ secrets.PEER_REVIEW_CHECKER_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| repositories: | | ||
| ${{ github.event.repository.name }} | ||
| GitHub-Actions | ||
| permission-administration: read | ||
| permission-contents: read | ||
| permission-members: read | ||
| permission-metadata: read | ||
| permission-pull-requests: read | ||
|
roryabraham marked this conversation as resolved.
|
||
|
|
||
| - name: Checkout repos | ||
| id: repo | ||
| uses: Expensify/GitHub-Actions/checkoutRepoAndGitHubActions@main | ||
|
|
||
| # v6.1.0 | ||
| - name: Setup Node | ||
| uses: actions/setup-node@395ad3262231945c25e8478fd5baf05154b1d79f | ||
| with: | ||
| node-version-file: GitHub-Actions/.nvmrc | ||
| cache: npm | ||
| cache-dependency-path: GitHub-Actions/package-lock.json | ||
|
|
||
| - name: Install npm packages | ||
| run: npm ci | ||
| working-directory: GitHub-Actions | ||
|
|
||
| - name: Verify peer review | ||
| run: >- | ||
| npm run verify-peer-review -- | ||
| --owner ${{ github.repository_owner }} | ||
| --repo ${{ github.event.repository.name }} | ||
| --pull-request-number ${{ github.event.pull_request.number }} | ||
| --base-ref ${{ github.event.pull_request.base.ref }} | ||
| working-directory: GitHub-Actions | ||
| env: | ||
| GITHUB_TOKEN: ${{ steps.generateAppToken.outputs.token }} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 24 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package.json | ||
| package-lock.json | ||
| *.yml | ||
| *.yaml | ||
| *.md | ||
| *.markdown | ||
| dist/ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import {createRequire} from 'node:module'; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
|
|
||
| /** @type {import('prettier').Config} */ | ||
| export default { | ||
| tabWidth: 4, | ||
| singleQuote: true, | ||
| trailingComma: 'all', | ||
| bracketSpacing: false, | ||
| arrowParens: 'always', | ||
| printWidth: 190, | ||
| singleAttributePerLine: true, | ||
| plugins: [require.resolve('@prettier/plugin-oxc'), require.resolve('@trivago/prettier-plugin-sort-imports')], | ||
| importOrderParserPlugins: ['typescript'], | ||
| importOrderImportAttributesKeyword: 'with', | ||
| importOrder: ['^[./]'], | ||
| importOrderSortSpecifiers: true, | ||
| importOrderCaseInsensitive: true, | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| import jestConfig from 'eslint-config-expensify/jest'; | ||
| import nodeConfig from 'eslint-config-expensify/node'; | ||
| import scriptsConfig from 'eslint-config-expensify/scripts'; | ||
| import tsConfig from 'eslint-config-expensify/typescript'; | ||
| import {defineConfig, globalIgnores} from 'eslint/config'; | ||
| import rulesdir from 'eslint-plugin-rulesdir'; | ||
| import {createRequire} from 'node:module'; | ||
| import path from 'node:path'; | ||
| import {fileURLToPath} from 'node:url'; | ||
|
|
||
| const projectRoot = path.dirname(fileURLToPath(import.meta.url)); | ||
| const require = createRequire(import.meta.url); | ||
| const expensifyConfigDirectory = path.dirname(require.resolve('eslint-config-expensify')); | ||
|
|
||
| rulesdir.RULES_DIR = path.resolve(expensifyConfigDirectory, 'eslint-plugin-expensify'); | ||
|
|
||
| export default defineConfig([ | ||
| globalIgnores(['node_modules']), | ||
| { | ||
| plugins: { | ||
| rulesdir, | ||
| }, | ||
| }, | ||
| ...nodeConfig, | ||
| ...tsConfig, | ||
| ...scriptsConfig, | ||
| ...jestConfig, | ||
| { | ||
| files: ['**/*.ts'], | ||
| languageOptions: { | ||
| parserOptions: { | ||
| project: path.resolve(projectRoot, 'tsconfig.json'), | ||
| projectService: false, | ||
| }, | ||
| }, | ||
| rules: { | ||
| // node:test is used instead of Jest in this repo. | ||
| 'jest/no-jest-import': 'off', | ||
| }, | ||
| }, | ||
| ]); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.