Add a Github action to deploy #1666
Conversation
| @@ -2,5 +2,6 @@ MANIFEST | |||
| .DS_Store | |||
| _book | |||
| node_modules | |||
| package-lock.json | |||
There was a problem hiding this comment.
that should be tracked in the repository to ensure we have deterministic build
There was a problem hiding this comment.
I considered that, but I generally hate having such huge files in a repo. You just end up with a lot more maintenance to keep it up to date. It also greatly pollutes your git log.
I'd like to get some more opinions from people around this. Normally I stay as far away from NPM and the Javascript ecosystem as I can but here I recognize we don't want to rewrite the entire tutorial.
There was a problem hiding this comment.
Without that file, we're exposing ourselves into breaking changes when anything upgrades (which actually will increase the maintenance rate, at least forced maintenance).
To keep it up to date we should probably introduce something like @dependabot (which will also check on every pull request that upgraded dependencies will pass our "tests").
Leaving it without package-lock.json we're asking for failing master builds for no reason.
There was a problem hiding this comment.
Right now we just install the latest gitbook (legacy). The plugins are also not all pinned. Not sure how much difference it would be since we've hardly seen build failures (AFAIK).
There was a problem hiding this comment.
I'd like to get some more opinions from people around this.
TL;DR: Do track package-lock.json in Git.
Here's my arguments for it in detail (mostly the same as what @magul already mentioned):
I considered that, but I generally hate having such huge files in a repo. [...] It also greatly pollutes your git log.
Pinning versions with a lock file might not be pretty, but it's a necessity.
You just end up with a lot more maintenance to keep it up to date.
We're using the software in question in our build process. If it isn't always the latest and greatest, no sweat. This won't usually cause any security issues for us. If it causes security issues for GitHub actions, GitHub will probably tell us.
On the other hand, we can probably blindly bump the versions to the latest whenever we feel like it and just see whether the result still works. The upside of having the lock file checked in, is, that whenever it turns out to not work as intended, we have known-working sets of versions that we can roll back to. Without that, we'd be in a situation of knowing it once worked but probably without any idea how to get it back to that state until upstream has fixed all issues and until we've eliminated all incompatibilities of our content with the new versions.
However rare that might be, having a lock file around for these situations will be well worth its downsides.
Right now we just install the latest gitbook (legacy). The plugins are also not all pinned.
That's fine if the now-latest versions work for us. (And it seems they do.) Whenever other latest versions are around that might not work, we'll want to be able to go back to these now-latest (then not-anymore-latest) known working versions.
Lock files aren't usually used to carefully curate what versions are to be used. (That's what version ranges and other constraints in package.lock or equivalent files are for, if one chooses to do that.) They're there to be able to roll back to (or simply stay on) known-working version combinations, whatever they might be, whenever the latest versions aren't working.
ekohl
force-pushed
the
replace-gitbook-legacy
branch
from
bfc14cc to
09ba532
Compare
This replaces Travis as the integration tool with Github Actions. The benefit is that it can also deploy to the gh-pages branch. This can then be deployed automatically after merge. It also switches from Gitbook legacy to Honkit. Since Github legacy is being phased out, this is needed.
ekohl
force-pushed
the
replace-gitbook-legacy
branch
from
09ba532 to
f7df500
Compare
| @@ -2,5 +2,6 @@ MANIFEST | |||
| .DS_Store | |||
| _book | |||
| node_modules | |||
| package-lock.json | |||
There was a problem hiding this comment.
I'd like to get some more opinions from people around this.
TL;DR: Do track package-lock.json in Git.
Here's my arguments for it in detail (mostly the same as what @magul already mentioned):
I considered that, but I generally hate having such huge files in a repo. [...] It also greatly pollutes your git log.
Pinning versions with a lock file might not be pretty, but it's a necessity.
You just end up with a lot more maintenance to keep it up to date.
We're using the software in question in our build process. If it isn't always the latest and greatest, no sweat. This won't usually cause any security issues for us. If it causes security issues for GitHub actions, GitHub will probably tell us.
On the other hand, we can probably blindly bump the versions to the latest whenever we feel like it and just see whether the result still works. The upside of having the lock file checked in, is, that whenever it turns out to not work as intended, we have known-working sets of versions that we can roll back to. Without that, we'd be in a situation of knowing it once worked but probably without any idea how to get it back to that state until upstream has fixed all issues and until we've eliminated all incompatibilities of our content with the new versions.
However rare that might be, having a lock file around for these situations will be well worth its downsides.
Right now we just install the latest gitbook (legacy). The plugins are also not all pinned.
That's fine if the now-latest versions work for us. (And it seems they do.) Whenever other latest versions are around that might not work, we'll want to be able to go back to these now-latest (then not-anymore-latest) known working versions.
Lock files aren't usually used to carefully curate what versions are to be used. (That's what version ranges and other constraints in package.lock or equivalent files are for, if one chooses to do that.) They're there to be able to roll back to (or simply stay on) known-working version combinations, whatever they might be, whenever the latest versions aren't working.
|
Just wanted to let you know I've seen this and I'll try to find time to get through the conversations and code this week and make relevant changes for DNS when we merge 🙌 |
aniav
dismissed
das-g’s stale review
The required changes were addressed already. All good.
|
Ah, it's actually not that easy with DNS, as my account on Django Girls 1pass got suspended. Will update the DNS when I get access to it 🙌 |
|
@das-g ah, sorry, @magul just made me aware of what happened, that your requested changes didn't actually get addressed. So to add to the voices here already: yes, we should lock the packages versions and keep them in the repository. For now, it's not a huge issue that I merged the PR without that change, I will make sure to apply this, so I'm not reverting. Just wanted to say I didn't ignore anyone intentionally 🙌 |
|
I'll try to get back to it, but wouldn't mind if someone else beats me to it. It at least looks like it correctly deploys to @aniav you can also disable Travis in case you didn't already. |
|
Travis disabled. Domain enabled 👌 |
|
Looks like it was migrated correctly and is now served via Github Pages. |
|
🎉 🎉 🎉 |
This replaces Travis as the integration tool with Github Actions. The benefit is that it can also deploy to the gh-pages branch. This can then be deployed automatically after merge.
It also switches from Gitbook legacy to Honkit. Since Github legacy is being phased out, this is needed.
This will address most of #1652. After it's merged, the admins still need to do set up of the Github Pages. Most importantly a DNS change.
I only did a quick scan of the result (see https://ekohl.github.io/tutorial/) and it looks good to me. It also has some nice additional features (dark mode, quick language switcher). A more thorough read through might be in order though.