Skip to content

Change PR deployment to build artifact + deploy#1576

Merged
nguyentvan7 merged 16 commits into
masterfrom
van/prDeploy
Feb 29, 2024
Merged

Change PR deployment to build artifact + deploy#1576
nguyentvan7 merged 16 commits into
masterfrom
van/prDeploy

Conversation

@nguyentvan7

@nguyentvan7 nguyentvan7 commented Feb 29, 2024

Copy link
Copy Markdown
Collaborator

Describe your changes

  • Changes described below should allow forks to have proper deployments while also deploying from a correct merge commit, unlike previous deployment implementations.
  • Change PR deployment to use artifact-based system
  • Build pipelines are triggered on PRs using pull_request event
    • This event has no secret access for forks, and requires contributor approval based on the repo settings.
    • This event will also wait to trigger until the mergeability check is complete and there is a merge commit available.
  • Once the build is complete, it will archive the output.
  • The completion of the build pipeline will trigger a separate deployment pipeline (this will not be shown in the Checks of a PR)
  • Deployment pipeline runs from our master branch, and has full secret permissions.
  • Deployment pipeline will download the artifact, push the contents to our deployment repo, and spawn/edit a comment on the original PR
  • Moved the clean pipeline to use pull_request_target.
    • This event has secret access for forks, and does not require approval.
    • This event always runs the workflow contents from master branch, so a developer cannot hijack our pipeline.
    • This event will trigger before the mergeability check is complete.
    • This isn't an issue because we don't need the merge commit for this to run, we just need secret permissions.

Issue or discord link

Testing/validation

Tested on my own repo nguyentvan7#27
Deployment succeeded, comment appeared as expected. We won't be able to test the fork-based capabilities until this is merged into the main repo. But at the very least, it is not a regression for non-fork branches.

Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)

  • I have commented my code in hard-to understand areas.
  • I have made corresponding changes to README or wiki.
  • For front-end changes, I have updated the corresponding English translations.
  • I have run yarn run mini-ci locally to validate format and lint.
  • If I have added a new library or app, I have updated the deployment scripts to ignore changes as needed

@nguyentvan7 nguyentvan7 requested a review from frzyc February 29, 2024 07:53
@nguyentvan7 nguyentvan7 marked this pull request as ready for review February 29, 2024 07:54
frzyc
frzyc previously approved these changes Feb 29, 2024

@frzyc frzyc left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

LGTM, looks like it works on your own repo as well. One minor nit.

Comment thread .github/workflows/deploy-frontend.yml Outdated
Comment thread .github/workflows/build-frontend.yml Outdated
@frzyc frzyc requested a review from eeeqeee February 29, 2024 08:06
nguyentvan7 and others added 2 commits February 29, 2024 01:09
Co-authored-by: eeeqeee <103794572+eeeqeee@users.noreply.github.com>
@nguyentvan7 nguyentvan7 requested a review from frzyc February 29, 2024 16:34
@nguyentvan7 nguyentvan7 merged commit af59815 into master Feb 29, 2024
@nguyentvan7 nguyentvan7 deleted the van/prDeploy branch February 29, 2024 19:48
RobberToe added a commit to RobberToe/genshin-optimizer that referenced this pull request Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert PR deployment pipeline to be a build -> upload drop -> deploy pipeline

3 participants