Skip to content

feat(ci): Adding required migration approval from codeowners#64223

Closed
IanWoodard wants to merge 13 commits into
masterfrom
iw/migration-check
Closed

feat(ci): Adding required migration approval from codeowners#64223
IanWoodard wants to merge 13 commits into
masterfrom
iw/migration-check

Conversation

@IanWoodard

Copy link
Copy Markdown
Contributor

Updating the GitHub workflows to require approval from owners-migrations before merging a migration.

@IanWoodard IanWoodard requested review from a team as code owners January 30, 2024 22:27
@github-actions github-actions Bot added Scope: Frontend Automatically applied to PRs that change frontend components Scope: Backend Automatically applied to PRs that change backend components labels Jan 30, 2024
@github-actions

Copy link
Copy Markdown
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@codecov

codecov Bot commented Jan 30, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (79a3c5e) 81.27% compared to head (683bbbc) 81.27%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #64223   +/-   ##
=======================================
  Coverage   81.27%   81.27%           
=======================================
  Files        5233     5233           
  Lines      230458   230458           
  Branches    45206    45206           
=======================================
+ Hits       187305   187308    +3     
+ Misses      37315    37312    -3     
  Partials     5838     5838           

see 6 files with indirect coverage changes

Comment on lines +47 to +52
- name: getsentry token
uses: getsentry/action-github-app-token@97c9e23528286821f97fba885c1b1123284b29cc # v2.0.0
id: getsentry
with:
app_id: ${{ vars.SENTRY_INTERNAL_APP_ID }}
private_key: ${{ secrets.SENTRY_INTERNAL_APP_PRIVATE_KEY }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will fail for external contributors -- but that's probably fine

Comment on lines +36 to +45
check-migration-approval:
name: check if migration is approved
runs-on: ubuntu-22.04
timeout-minutes: 3
needs: did-migration-change
if: needs.did-migration-change.outputs.added == 'true'
steps:
- uses: actions/checkout@93ea575cb5d8a053eaa0ac8fa3b40d7e05a33cc8 # v3.1.0
with:
persist-credentials: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this won't work. it needs to re-trigger on approval somehow -- this only runs on push

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I was just using this as a means of testing. I am planning on moving this to a separate workflow 👍🏻

@IanWoodard IanWoodard closed this Jan 31, 2024
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants