-
Notifications
You must be signed in to change notification settings - Fork 65
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
Consolidate backend checks #19126
Closed
Closed
Consolidate backend checks #19126
Changes from 81 commits
Commits
Show all changes
83 commits
Select commit
Hold shift + click to select a range
f2c63a8
rename files for clarity/add notes
ryan-mcneil 1f8dff7
replace rspec run with single spec for testing
ryan-mcneil f93499c
add backend-pr-labeler
ryan-mcneil d32d8b4
disable old checks
ryan-mcneil c623e10
add mobile comment
ryan-mcneil df89103
fix code checks and export var
ryan-mcneil cf4dd36
run full specs + add 'labeled' option
ryan-mcneil b6b801d
test exit 1 and echo pr info
ryan-mcneil ec532ee
Add require-be label
ryan-mcneil 4fa36ff
add curly braces
ryan-mcneil 0a3b415
fix draft check
ryan-mcneil 69f42a4
remove backend label
ryan-mcneil a4cfbfa
add ready-for-backend-review from old job with small mods
ryan-mcneil 42935d4
correctly use outputs
ryan-mcneil bc6ba71
add better logging
ryan-mcneil 70e411c
Merge branch 'master' into rm-consolidate-backend-checks
ryan-mcneil 56c2bdf
fix if statement format
ryan-mcneil 197d6bf
adjust failure check logic
ryan-mcneil 818fede
adjust logging
ryan-mcneil 3cac44d
fix id
ryan-mcneil 70d09b8
more output
ryan-mcneil 8b46a7b
correctly use 'booleans'
ryan-mcneil 19b1bd4
remove unnecessary if statements
ryan-mcneil bbd1e93
debug label logic
ryan-mcneil 620de09
Merge branch 'master' into rm-consolidate-backend-checks
ryan-mcneil 63dd1b7
fix codechecks and remove unecessary labels
ryan-mcneil 9a187aa
run labeler after code_checks
ryan-mcneil 295ae5b
fix error in codeowners check
ryan-mcneil b658265
Merge branch 'master' into rm-consolidate-backend-checks
ryan-mcneil cb1252b
fix if syntax
ryan-mcneil 8bea003
add synchronize to test
ryan-mcneil 235bb5c
debugging
ryan-mcneil 356b177
again
ryan-mcneil f4d1860
remove sync
ryan-mcneil b0f2f92
fix ==
ryan-mcneil ee0b631
debugging
ryan-mcneil 2dc75fd
indentation :(
ryan-mcneil ff37ff2
Try running without specs
ryan-mcneil 887945a
remove steps and completed type
ryan-mcneil 3db8c7d
codechecks logging
ryan-mcneil 2081c15
merge master
ryan-mcneil 56b5e7f
Trigger build
ryan-mcneil cb8c3c8
add pull_request_review type, parse pull request, move test labels
ryan-mcneil 008a821
add be pr approval
ryan-mcneil 51f7f80
fix pr grab
ryan-mcneil d9c8e6b
convert to JSON to store object
ryan-mcneil 29020ff
correctly set pull_reqeust
ryan-mcneil dceb9e6
fix pull_request usage
ryan-mcneil a671abe
fix/refactor env vars
ryan-mcneil a44777e
fix labels env var
ryan-mcneil e1825a3
Used AI to do a bit of ENV refactoring
ryan-mcneil d52c120
back before AI
ryan-mcneil 4726d20
retrieve pr values once
ryan-mcneil b264d8c
fix array JSON
ryan-mcneil 53d0886
use different quotes
ryan-mcneil b581b4b
Try Claude rec
ryan-mcneil 5e2939f
remove broken debugging lines
ryan-mcneil 3df4335
fix failure label comparison
ryan-mcneil f2ab5de
log PR data
ryan-mcneil a54514c
remove spaces after ||
ryan-mcneil 411d240
try without square brackets
ryan-mcneil 347b157
fix draft var
ryan-mcneil 370f621
Update labeling notes/requirements
ryan-mcneil 658055d
merge master
ryan-mcneil 4c20dde
retrieve entire pr object
ryan-mcneil 163d600
Merge branch 'master' into rm-consolidate-backend-checks
ryan-mcneil c652466
update for Code Checks running on push to master
ryan-mcneil 5385db0
Use pull_request structure for pull_request_review
ryan-mcneil 92bf1fe
update logging
ryan-mcneil 0493802
merge master
ryan-mcneil 43cfab4
merge master
ryan-mcneil d92ce59
Add extra job to filter on:push events
ryan-mcneil 5f656f2
merge master
ryan-mcneil 8e3a7fc
Comment out unused test-status code
ryan-mcneil 56df7bb
fix get-pr-data var
ryan-mcneil 9a2e3de
Merge branch 'master' into rm-consolidate-backend-checks
ryan-mcneil 825ab22
Clean up labeler
ryan-mcneil 599ba0e
Fix label logic
ryan-mcneil d9f9b7a
Clean up code checks
ryan-mcneil f60e1ac
remove comment
ryan-mcneil 072aeca
remove old files
ryan-mcneil 12451e9
few more things from code_checks
ryan-mcneil a290b27
update workflow name
ryan-mcneil 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 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 |
---|---|---|
@@ -1,13 +1,30 @@ | ||
name: Backend PR Approver | ||
on: | ||
workflow_run: | ||
workflows: | ||
- "Backend PR Labeler" | ||
types: [completed] | ||
pull_request: | ||
types: [labeled, unlabeled] | ||
jobs: | ||
say-hi: | ||
check-approval-labels: | ||
if: github.event.label.name == 'require-backend-approval' || github.event.label.name == 'ready-for-backend-review' | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Say Hi | ||
- name: Check for exemption | ||
id: check_exemption | ||
run: | | ||
echo "Hi" | ||
if [ ! ${{ contains(toJSON(github.event.pull_request.requested_teams.*.name), 'backend-review-group') }} ]; then | ||
echo "exempt=true" >> $GITHUB_OUTPUT | ||
echo "PR is exempt from backend approval." | ||
else | ||
echo "exempt=false" >> $GITHUB_OUTPUT | ||
echo "PR requires backend approval." | ||
fi | ||
|
||
- name: Check for backend approval | ||
id: check_approval | ||
if: ${{ steps.check_exemption.outputs.exempt == 'false' }} | ||
run: | | ||
if [ ${{ contains(toJSON(github.event.pull_request.labels.*.name), 'require-backend-approval') && contains(toJSON(github.event.pull_request.labels.*.name), 'ready-for-backend-review') }} ]; then | ||
echo "Pull Request missing approval." | ||
exit 1 | ||
else | ||
echo "Pull Request approved." | ||
fi |
This file contains 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 was deleted.
Oops, something went wrong.
This file contains 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 |
---|---|---|
|
@@ -64,7 +64,7 @@ jobs: | |
|
||
- name: Remove Failure label | ||
uses: actions-ecosystem/action-remove-labels@v1 | ||
if: ${{ success() }} | ||
if: ${{ success() && contains(github.event.pull_request.labels.*.name, 'codeowners-addition-failure') }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. notice a quiet failure caused by the workflow attempting to remove a label that wasn't there. |
||
with: | ||
number: ${{ github.event.pull_request.number }} | ||
labels: | | ||
|
This file contains 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this true? In CODEOWNERS, files either have backend-review-group listed, OR they have a lighthouse team, octo-identity, or mobile? 🤔 no... I just found this line:
spec/lib/token_validation @department-of-veterans-affairs/lighthouse-dash @department-of-veterans-affairs/lighthouse-banana-peels @department-of-veterans-affairs/va-api-engineers @department-of-veterans-affairs/backend-review-group
With the logic here, a PR that changes files in this dir would hit the
else
block.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the solution is to remove
backend-review-group
from those lines in codeowners 🤔There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be an example of a CODEOWNERS issue. If they own it, our team doesn't need to be on it. If we should be involved in the review, then it'll work as expected. I think these cases will iron themselves out. I can't think of a good reason for why both would be needed yet we shouldn't be a required approver.