lazerwalker / twine-app-builder

Automatically generate Windows and macOS versions of your Twine games, for free!
MIT License
252 stars 44 forks source link

fix: add hard-coded `ref` instead of checking out default branch #26

Closed seleb closed 3 years ago

seleb commented 3 years ago

Currently the checkout step will always use the latest version of the default branch of the template repo, which may or may not be compatible with older versions of the builder repo. In the future, you may end up making updates to the template that involve breaking changes for the builder, and although you can push a matching change to the builder right away, everyone else's workflows will break without warning (it's unlikely folks will be pulling upstream changes frequently, if at all).

Note: It may be worth looking into making the template either part of this repo, a self-contained action, or an executable npm module rather than manually checking it out as part of this project's workflow so that the versioning/dependency management is less of a concern.

lazerwalker commented 3 years ago

Thanks for this! I'd mentally written off thinking about proper update strategies as something to deal with later, but this is 1000% a reasonable and good thing to do for now.

I do want to base it off of a tag instead of a SHA, though -- since a huge part of the audience for this is folks unfamiliar with both git and GH, I think it's valuable having it be slightly friendlier and more human-readable (and also opens the door to eventually doing more semver-aware updates, although I suspect that'll be a pain). Instead of merging this as-is, once I'm done poking at / merging in your other changes I'll manually push a new tag to the other repo and update it in this repo. Leaving this PR open as a reminder until I do that :)

As an aside, I investigated all of those other options before shipping this. A self-contained action turned out to be a major hassle to build compared to this flow -- it might still be worth doing, but a lot of the filesystem structure within an executable context was not as well-documented as I needed for that to be a quick/easy port job when I was trying to just get this out the door. From there, I consciously moved the template project out of this repo into a separate repo, since keeping the bare repo as sparse as possible seemed valuable for less-technical users, and minimizes the extent to which framework-level updates will murk up individual projects' git history. I didn't seriously consider publishing an npm module, under the assumption that keeping things visible and easily hackable was a good thing. I've since gotten feedback from a handful of folks who used this project as a gentle onboarding to hacking the template and modifying the Electron project, which seems like a strong vote in favor of keeping the template code in a form that's as easily hackable as possible.

lazerwalker commented 3 years ago

Done!

seleb commented 3 years ago

I do want to base it off of a tag instead of a SHA, though

totally makes sense!

I didn't seriously consider publishing an npm module, under the assumption that keeping things visible and easily hackable was a good thing.

i agree that having the electron side of things accessible for hacking is a good thing, but i don't think these have to necessarily be mutually exclusive. atm, customizing electron requires two steps:

  1. fork the electron template
  2. edit your workflow to point at your template instead of the original

an npm-based setup could also be kept to the same two-step process without even introducing any files to the builder repo: a command like npx @lazerwalker/electron-template@v1.x could be used, which would replace the need for checking out the repo manually in order to run scripts inside it. this method wouldn't require a package.json, and since github repos can be treated as npm modules too, hacking it would still just require forking the original and pointing at your fork. you technically wouldn't even have to publish to the npm registry to support this method; you could npx your template repo directly (though i think that would make versioning and updates a bit more manual)

this could potentially improve the separation of concerns between the github workflow and the template project too; there's a lot of non-CI/CD-specific scripting in the workflow which could move to the hypothetical npx script instead and make the template more usable as a standalone piece

this would be a non-trivial refactor ofc, but some food for thought!