feat(install.sh): add support for --mainline (default) and --stable
#12858
Conversation
mafredri
force-pushed
the
mafredri/feat-install.sh-add-stable-mainline-support
branch
from
f68b2b5
to
339acb9
Compare
mafredri
force-pushed
the
mafredri/feat-install.sh-add-stable-mainline-support
branch
from
2d79f59
to
ace59df
Compare
| # separated numbers and without utilizing the sort -V flag for the | ||
| # best compatibility. | ||
| curl -fsSL https://api.github.com/repos/coder/coder/releases | | ||
| awk -F'"' '/"tag_name"/ {print $4}' | |
There was a problem hiding this comment.
nit: you didn't want to use jq? :)
There was a problem hiding this comment.
I’d love to, but it’s a dependency we can’t afford.
| @@ -106,9 +118,18 @@ echo_standalone_postinstall() { | |||
| return | |||
| fi | |||
|
|
|||
| channel=mainline | |||
| advisory="To install our stable release (v${STABLE_VERSION}), use the --stable flag. " | |||
There was a problem hiding this comment.
nit: the trailing space might not be most convenient form, but I see why you have added this way, one condition less.
There was a problem hiding this comment.
I initially had the entire advisory as two separate vars, then I noticed the repetition and extracted common parts 😅, I can go back to that method if you think it’s preferable.
There was a problem hiding this comment.
Probably not worth it, this is just a single condition :) Thanks for sharing more context!
install.sh
Outdated
| @@ -282,8 +303,12 @@ main() { | |||
| --version=*) | |||
| VERSION="$(parse_arg "$@")" | |||
| ;; | |||
| --edge) | |||
| EDGE=1 | |||
| # Support edge for backwards compatibility. | |||
There was a problem hiding this comment.
Both forms seem to be used, but perhaps “backward” is the more common one.
| # https://gist.github.com/lukechilds/a83e1d7127b78fef38c2914c4ececc3c#gistcomment-2758860 | ||
| version="$(curl -fsSLI -o /dev/null -w "%{url_effective}" https://github.com/coder/coder/releases/latest)" | ||
| version="${version#https://github.com/coder/coder/releases/tag/v}" | ||
| echo "${version}" |
There was a problem hiding this comment.
I think the upper/lower case is a good enough distinction, and this practice (case) is often used to separate script global and function local variables. Although inherently case doesn’t differentiate between scope, for that local/typeset/declare needs to be used. Since we’re supporting posix shell though, we don’t try to use such “advance features”. 😄
There was a problem hiding this comment.
Alright, we can always adopt local keyword :)
There was a problem hiding this comment.
| cath <<EOF | ||
|
|
||
| Coder has been installed to | ||
| Coder ${channel} release v${VERSION} installed. ${advisory}See our releases documentation or GitHub for more information on versioning. |
mafredri
deleted the
mafredri/feat-install.sh-add-stable-mainline-support
branch
This PR adds support for installing either the mainline or stable version of Coder.
The
--edgeflag was deprecated and--mainline(default) and--stableadded in it's place.The previous implementation for "edge" did not take into account that the release order might not be reliable, so the implementation was updated to perform version-based sorting on all the tags available in the JSON response for releases, and the highest version numer is selected.
In the future we may want to filter out releases on a certain criteria (e.g. exclude draft/prerelease), however, I decided to ommit this to avoid premature feature development and to keep the change simpler.
Note: I decided against embedding
CODER_MAINLINE_VERSIONin the script because this introduces added complexity in the release flow:CODER_MAINLINE_VERSIONand the GitHub Actions build completing, before build completion, the new version is not available, even if cutFixes #12461