-
Notifications
You must be signed in to change notification settings - Fork 45
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
Improve release workflow and readme #41
Conversation
I changed the environment variables and inputs to Strings instead of Booleans because GitHub doesn't allow more than 10 inputs in a manual workflow :/ (https://github.com/orgs/community/discussions/8774) This way, when we add more build or publish targets, we won't hit the limit. I also think this looks better. |
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.
Half of a review for now. I'm going to have to take a look at the docs later.
How does this behave when a user hasn't configured github pages or itch? (Opinion) I feel like if a user has chosen to deploy to GH pages and GH pages is not configured, then they should see an error that points in the right direction. Ditto itch. If that's the case though, I don't think it makes sense to deploy to either of those platforms by default. Users using the workflow for the first time would just be bombarded with errors. |
@rparrett You are right, It was set up that way, I just added itchio and pages for testing and forgot to remove them. |
What's the usecase for the manual workflow? Are users expected to set all the fields every time they use it, or to update the YAML to the settings they normally use? If it's the latter - wouldn't it be simpler and easier to use if the manual workflow only asks for the tag version, and does not allow modifying anything else? |
Well, I see environment variables as your default settings, which you want to maintain for every time you release a new version of your game. I consider the manual workflow more flexible to allow testing a platform without having to change your default configuration |
Looks good to me, but I worry this takes the workflow over a complexity threshold... Could you try splitting it in several files, using https://docs.github.com/en/actions/using-workflows/reusing-workflows? |
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Idan Arye <IdanArye@gmail.com>
8426500
to
6f739fb
Compare
@mockersf I update the pr, now every build and release has its own file using workflow calls |
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.
Leaving a few suggestions but feel free to ignore them if they provide little value vs the benefits we might get from them.
inputs: | ||
tag: | ||
required: true | ||
type: string | ||
os: | ||
required: true | ||
type: string | ||
artifact_name: | ||
required: true | ||
type: string |
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'd add description
fields for all inputs (in all scripts). That should make it clearer what they work for when you glance at the "internal" scripts. I see the manual workflows are properly documented so that's great but I guess it won't hurt to do it for these too. Let me know what do you think.
itchio_target: | ||
required: true | ||
type: string |
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.
This input is a great candidate to be documented with a description
IMHO.
@@ -0,0 +1,56 @@ | |||
name: Build Linux |
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 was going to suggest a matrix based, single script for builds but then I realized each one of these are so different that it might turn into spaghetti pretty quickly. 😅
3. If your project doesn't have an `assets` folder: | ||
1. You can create one and add a `.gitkeep` file to it to enable you to push it to your repository. | ||
2. Alternatively, if your project does not use assets, you can remove the `cp -r assets` statements from the build jobs in the workflow. |
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.
Would a "disable assets folder copy" input help with this? By default it would be false but users could turn it on and that step will not run. This way you don't need to alter the template for this use case. It could be too much for too little benefit though so feel free to ignore this.
|
||
Once that is done, any tag pushed to GitHub will trigger an itch.io release and use the tag as the [user version](https://itch.io/docs/butler/pushing.html#specifying-your-own-version-number). | ||
1. For web builds, ensure that your project includes an `index.html` file under the `/wasm` directory. | ||
2. Make sure that the name of your repository matches the name of your binary. If the name of your repository is different, simply change the env variable [`binary`](.github/workflows/release.yaml#L7) in the release.yaml file to match the name of your binary. |
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.
And for stuff like this, would GitHub vars
help to also avoid changing the script? This way you could default to the repo name if a variable is not found in the repository (something like ${{ vars.BINARY_NAME || github.repo }}
). This same suggestion might apply for some other places where default values could be overwritten without changing scripts.
This PR improves the release workflow by adding:
This also improves the README and addresses (#14).
I tested it, and it works in this repository:
https://github.com/ameknite/bevy_game
itch.io page:
https://ameknite.itch.io/bevy-game
github pages:
https://ameknite.github.io/bevy_game/