feat: Initial entry for a workflow to package the gui application.#1166
feat: Initial entry for a workflow to package the gui application.#1166maximearmstrong merged 15 commits intoMobilityData:masterfrom
Conversation
|
Per https://github.com/MobilityData/gtfs-validator/actions/runs/2382685953, this workflow is successfully packaging for Windows, Mac, and Linux. Ready for review. |
maximearmstrong
changed the title
maximearmstrong
left a comment
maximearmstrong
left a comment
There was a problem hiding this comment.
Two small comments before approval. Thanks @bdferris-v2
| - '**/package_app.yml' | ||
| release: | ||
| types: [ prereleased, released ] | ||
| workflow_dispatch: |
There was a problem hiding this comment.
Do we need the : here since we are not providing anything after?
There was a problem hiding this comment.
I think we do need it. I tried removing it and got a workflow error:
More importantly, it seems pretty common to include the : in other projects:
There was a problem hiding this comment.
Perfect, thank you!
|
Also, now that #1168 is merged, the acceptance tests should pass for a future commit. |
barbeau
left a comment
barbeau
left a comment
There was a problem hiding this comment.
Wow, I'm amazed that this was so straightforward 💯.
I tested the Windows installer that was uploaded and it works correctly 👍.
One small item in-line.
.github/workflows/package_app.yml
Outdated
| @@ -0,0 +1,50 @@ | |||
| name: Package Application | |||
There was a problem hiding this comment.
Recommend renaming this (and file name) so it's more clearly distinguished from other workflows that also "package" the app. Maybe:
| name: Package Application | |
| name: Package Installer |
There was a problem hiding this comment.
Seems reasonable. Done.
maximearmstrong
left a comment
maximearmstrong
left a comment
There was a problem hiding this comment.
One small thing aside, we are ready to merge :)
isabelle-dr
left a comment
isabelle-dr
left a comment
There was a problem hiding this comment.
I believe we could update README.md in this PR to add the instructions?
We could add a section "Run the validator app" describing (1) how to download the app and (2) how to use it.
If we want to mirror what we have with the JAR file, we will also need to include a DOWNLOAD_SNAPSHOT_APP.md similar to DOWNLOAD_SNAPSHOT_JAR.md.
|
@isabelle-dr I'm happy to write that documentation, but I think it's getting beyond the original scope of this PR? Would it be ok if I updated documentation in a follow-up PR? |
|
@bdferris-v2 No problem updating the documentation in another PR, I thought it was in scope. |
|
We just need to update this branch with |
|
Done. |
|
Thank you @bdferris-v2 ! |
4 participants
Implementation for a GitHub workflow to build the GUI application installers and packages on all key environments.