Skip to content
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: Also build CD on PRs #5021

Merged
merged 2 commits into from
May 2, 2020
Merged

feat: Also build CD on PRs #5021

merged 2 commits into from
May 2, 2020

Conversation

MCOfficer
Copy link
Collaborator

@MCOfficer MCOfficer commented Apr 26, 2020

Feature: This PR implements enables CD builds on Pull Requests, so people can download and playtest PRs without having to compile.

Feature Details

With this change, the CD workflow will run,

  1. when a new commit is pushed to the master branch
  2. when a PR is opened
  3. when new commit are added to a PR

In the case of 1), artifacts will be published to the continuous release. For PRs, artifacts can be downloaded from the "Checks" tab:
https://gfycat.com/annualtintedcopepod

The download is still only available for logged-in users: actions/upload-artifact#51

Testing Done

Tested on a multitude of PRs. As far as i can tell, this change needs to be in the head branch, so this will not work for old PRs until they merge master.

Follow-up features:

Post a comment on the PR with links to the assets. Unfortunately, Github's Actions API is still somewhat immature, so this would require querying and parsing the Github API. There is a nice action for "Sticky" comments, though: https://github.com/marocchino/sticky-pull-request-comment

@MCOfficer
Copy link
Collaborator Author

Here's some more incentive: (If a PR includes this change,) everyone can playtest it with minimal effort.

I already implemented this into the ESLauncher2 prototype. It uses a workaround to download artifacts despite not having a Github account:
ESLauncher2-PR-Demo

@tehhowch
Copy link
Collaborator

tehhowch commented Apr 27, 2020

Sounds reasonable. Since we'll be doing more on each commit there are some things we should also do:

  1. shared prefix (e.g. cd or pkg) for CD jobs
  2. indicate compatibility (e.g. MacOS 10.15) where needed.
  3. Cancel pending / running workflows whenever a new commit is added to a PR, to avoid redundant work. I think this will require using the REST API: https://developer.github.com/v3/actions there may be some marketplace actions too

@MCOfficer
Copy link
Collaborator Author

MCOfficer commented Apr 27, 2020

I don't get 1). The workflow is named "CD", and all jobs reference their workflow.
image

  1. i agree with, but i'm not sure where to put in a PR. It's easy once actions can post to the PR, but as i said, the API is too immature for that (Support artifact URL output actions/upload-artifact#50). We can at least mark it on the releases page, but that has little to do with this PR (rather Continuous Build Crashing When Opened (Mac) #5013).

  2. Is reasonable.

@tehhowch
Copy link
Collaborator

I suggested 1 due to the mobile page view:

screen cap

Screenshot_20200427-140010

For 2 I was thinking to just name the output accordingly.

@tehhowch
Copy link
Collaborator

tehhowch commented May 2, 2020

We can add run cancelling as a follow-up.

@tehhowch tehhowch merged commit 6b705fd into endless-sky:master May 2, 2020
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.

2 participants