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

Improve release workflow and readme #41

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

ameknite
Copy link
Contributor

@ameknite ameknite commented Sep 19, 2023

This PR improves the release workflow by adding:

  • Manual workflow dispatch
  • Setting environment variables to specify the desired build and release platforms.
  • Separating build and release jobs
  • Adding deployment to GitHub Pages.

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/

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@ameknite
Copy link
Contributor Author

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.

Screenshot 2023-09-21 at 14 33 24

Copy link
Contributor

@rparrett rparrett left a 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.

.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
.github/workflows/release.yaml Outdated Show resolved Hide resolved
@rparrett
Copy link
Contributor

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.

@ameknite
Copy link
Contributor Author

@rparrett You are right, It was set up that way, I just added itchio and pages for testing and forgot to remove them.
Fixed.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
.github/workflows/ci.yaml Outdated Show resolved Hide resolved
@idanarye
Copy link
Contributor

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?

@ameknite
Copy link
Contributor Author

ameknite commented Sep 23, 2023

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

@mockersf
Copy link
Member

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?

@ameknite
Copy link
Contributor Author

ameknite commented Oct 17, 2023

@mockersf I update the pr, now every build and release has its own file using workflow calls

Copy link
Member

@mnmaita mnmaita left a 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.

Comment on lines +5 to +14
inputs:
tag:
required: true
type: string
os:
required: true
type: string
artifact_name:
required: true
type: string
Copy link
Member

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.

Comment on lines +9 to +11
itchio_target:
required: true
type: string
Copy link
Member

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
Copy link
Member

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. 😅

Comment on lines +148 to +150
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.
Copy link
Member

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.
Copy link
Member

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.

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.

None yet

7 participants