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

Add CD via Github Actions #351

Merged
merged 8 commits into from
Jul 1, 2020
Merged

Add CD via Github Actions #351

merged 8 commits into from
Jul 1, 2020

Conversation

MCOfficer
Copy link
Contributor

Closes #93

This PR adds a Github Actions workflow that builds binaries and publishes them on the workflow page. If the workflow runs on a tag, it will also publish to the releases page.

The workflow runs on every new push, tag or release.

@iceiix
Copy link
Owner

iceiix commented Jul 1, 2020

Cool, thanks! This looks great, I guess it replaces the Azure Pipeline I added in #346, but it is better since it adds publishing binary releases. I see it also uses a static CRT for Windows, implementing #318.

I'll merge this as-is but we should see about combining the functionality of azure-pipelines.yml into .github/workflows/build.yaml, haven't looked at it in depth but a few preliminary notes:

  • stripping symbols: not sure this is needed at this stage, having symbols in the released binary could be useful for debugging end-user reports
  • clippy: in azure-pipelines, does not appear in the GitHub actions, but it would be useful to have. It can't be enabled with a hard deny yet until Fix clippy warnings #347, but could still run and warn but not fail
  • macos binary: stevenarella-macos-latest.zip is zipped, extracting reveals a single file: stevenarella-macos-latest. Double-clicking opens in TextEdit. To run as an executable, it should at least have the executable bit set (chmod a+x). Linux should also have this.
  • macos app: even better, would be to bundle as an .app, but this would require more changes to create the app bundle properly (moving the binary in Contents/MacOS/, maybe adding an icon, etc.)
  • naming: would be nice to simplify the filenames to stevenarella.exe (Windows), Stevenarella.app (macOS, app bundle folder), stevenarella (Linux), removing -platform-latest (is it possible to remove the zip archiving too, at least for Windows/Linux? macOS would need .zip if bundled into an .app bundle), but need to ensure we have another way to identify the version (embed in the binary? -v/--version flag?)

@iceiix iceiix merged commit 6b13fa3 into iceiix:master Jul 1, 2020
@MCOfficer
Copy link
Contributor Author

MCOfficer commented Jul 1, 2020

I'll merge this as-is but we should see about combining the functionality of azure-pipelines.yml into .github/workflows/build.yaml, haven't looked at it in depth but a few preliminary notes:

Should probably open a tracking issue for this, lest i forget.

  • stripping symbols: not sure this is needed at this stage, having symbols in the released binary could be useful for debugging end-user reports

It helps a lot with final executable size. How about also uploading a debug build - as far as i'm aware we have to build in debug anyways for clippy to work.

  • clippy: in azure-pipelines, does not appear in the GitHub actions, but it would be useful to have. It can't be enabled with a hard deny yet until Fix clippy warnings #347, but could still run and warn but not fail

Agreed, the big question is should it be part of the same job, or run separately?

  • macos binary: stevenarella-macos-latest.zip is zipped, extracting reveals a single file: stevenarella-macos-latest. Double-clicking opens in TextEdit. To run as an executable, it should at least have the executable bit set (chmod a+x). Linux should also have this.

  • macos app: even better, would be to bundle as an .app, but this would require more changes to create the app bundle properly (moving the binary in Contents/MacOS/, maybe adding an icon, etc.)

The executable flags are a problem (actions/upload-artifact#38 among others) - unless one archives their executables before uploading, executable bits will be reset. For linux, i don't see it as much of a problem; linux users are savvy enough to chmod. MacOS should be done by just packing an app, that has to be archived anyways.
Fortunately, pega3 just contributed a working MacOS workflow to one of my projects, so we're free to steal from there: EndlessSkyCommunity/ESLauncher2#42

  • naming: would be nice to simplify the filenames to stevenarella.exe (Windows), Stevenarella.app (macOS, app bundle folder), stevenarella (Linux), removing -platform-latest (is it possible to remove the zip archiving too, at least for Windows/Linux? macOS would need .zip if bundled into an .app bundle), but need to ensure we have another way to identify the version (embed in the binary? -v/--version flag?)

Github actions always zips by default(actions/upload-artifact#39), there's no way around that. The releases page is unaffected, see https://github.com/MCOfficer/stevenarella/releases/tag/test4

As for the version, one can put it in the window title.

PS: Thanks for the invitation, but i rejected - i don't see much to gain since i would be pushing PRs anyways, and the risk of me pushing to the wrong remote is too high in my book. Let's just say it happened before.

@iceiix
Copy link
Owner

iceiix commented Jul 2, 2020

Filed #352 for future enhancements (can be fixed separately)

PS: Thanks for the invitation, but i rejected

No worries, thank you regardless for your contributions!

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.

Publish binary releases
2 participants