-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: preview label deployments #1413
Conversation
Yes! Thank you! |
const event = ${{ toJson(github.event) }} | ||
const eventName = '${{ github.event_name }}' | ||
|
||
let name = '' |
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 necessary? does using the branch name not work?
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.
let me check, I think it's because the ref is different depending on the event but maybe we can use event.ref
for both 🤔
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.
.github/workflows/dev-deploy.yml
Outdated
github.event_name == 'pull_request' && ( | ||
github.event.action == 'labeled' || ( | ||
github.event.action == 'synchronize' && | ||
contains(github.event.pull_request.labels.*.name, 'preview') |
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 the only thing you need to check? this whole condition only has to be a subset of the condition at the job level right?
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 the main thing is we only want this to run when:
- it's a push to
dev-*
- the PR is labeled
- the PR is updated
we actually do check if preview
exists before executing the job here:
so maybe we can remove this since it's already checked. if the event is synchronize
, then it should have the preview
label since the above condition passed 🤣
let me check and see if it works as expected without this
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.
just checked, yeah this can be removed. thank you for catching this!! 🔥
|
||
https://${{ steps.deploy-data.outputs.result }}.cryoet.dev.si.czi.technology | ||
|
||
Updated: ${{ steps.get-comment-date.outputs.result }} PST |
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.
Fancy!
Adds the ability to create preview deployments from an existing PR by adding a
preview
label to the PR. This works by updating thedev-deploy.yml
so that:preview
label creates a new preview deployment and posts a comment on the PR with the URL and last updated datepreview
label takes down preview deployment and deletes the PR commentChanges to existing workflow
dev-*
should still work as expectedDemo
PR comment
Create preview on label
https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12418548047
Delete preview on label
https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12405421842
Delete preview on close
https://github.com/chanzuckerberg/cryoet-data-portal/actions/runs/12421062766