-
Notifications
You must be signed in to change notification settings - Fork 0
Add Brioche nightly support #11
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
Conversation
|
@kylewlacy I put this PR in draft, since I would like first to run the tests (see this comment for context: #10 (comment)), I think we need to tweak the settings of this GitHub repository |
Signed-off-by: Jérémy Audiger <[email protected]>
…using nightly channel Signed-off-by: Jérémy Audiger <[email protected]>
Signed-off-by: Jérémy Audiger <[email protected]>
e422065 to
ef2e061
Compare
Signed-off-by: Jérémy Audiger <[email protected]>
b2fee41 to
d57a203
Compare
Signed-off-by: Jérémy Audiger <[email protected]>
d57a203 to
57718ba
Compare
Signed-off-by: Jérémy Audiger <[email protected]>
…ual latest version of Brioche Signed-off-by: Jérémy Audiger <[email protected]>
.github/workflows/test.yml
Outdated
| - runs-on: ubuntu-latest | ||
| channel: stable | ||
| - runs-on: ubuntu-latest | ||
| channel: nightly | ||
| - runs-on: ubuntu-latest-arm | ||
| channel: stable | ||
| - runs-on: ubuntu-latest-arm | ||
| channel: nightly |
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.
| - runs-on: ubuntu-latest | |
| channel: stable | |
| - runs-on: ubuntu-latest | |
| channel: nightly | |
| - runs-on: ubuntu-latest-arm | |
| channel: stable | |
| - runs-on: ubuntu-latest-arm | |
| channel: nightly | |
| - runs-on: ubuntu-24.04 | |
| channel: stable | |
| - runs-on: ubuntu-24.04 | |
| channel: nightly | |
| - runs-on: ubuntu-24.04-arm | |
| channel: stable | |
| - runs-on: ubuntu-24.04-arm | |
| channel: nightly |
Just found out that GitHub Actions doesn't seem to support ubuntu-latest-arm (and they don't plan to add it). I think that's kinda annoying... so it seems sensible to just use a specific Ubuntu version for each of these jobs (we could keep using ubuntu-latest but to keep it symmetric I'd lean towards keeping all of them pinned together)
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.
Ok, I updated it in c4060fa. And I took the opportunity to re-add ubuntu-latest in the matrix of the smoke test
README.md
Outdated
| uses: brioche-dev/setup-brioche@v1 | ||
| # with: | ||
| # version: "v0.1.5" # Optional | ||
| # version: "latest" # Optional |
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.
Minor tweak to show the separation between version and channel in the README
| # version: "latest" # Optional | |
| # version: "v0.1.5" # Optional | |
| # channel: "latest" # Optional |
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.
Kinda done, since it's now removed
|
@jaudiger One idea I started thinking about... what if we got rid of Again just an idea, so I'm fine with keeping them separate if you think it's better this way! |
|
Well, there is pro and cons depending on the chosen options:
I would prefer the use of two field since it's clearer to know what fields mean, but I'm not against merging them. Both possibilities are valid and quite common. But I tend to agree that if one day, we want to extend the GitHub Action to also choose the nightly version, we cannot keep the field jobs:
setup-brioche:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v5
- name: Setup Brioche
uses: brioche-dev/setup-brioche@v1
with:
channel: 'nightly'
version: '2025-10-02' # It's kinda weird to have a date here, and it could confuse the end user, since this version does not really existSo, we could either rename jobs:
setup-brioche:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v5
- name: Setup Brioche
uses: brioche-dev/setup-brioche@v1
with:
ref: 'nightly@2025-10-02'Versus: jobs:
setup-brioche:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v5
- name: Setup Brioche
uses: brioche-dev/setup-brioche@v1
with:
channel: 'nightly'
ref: '2025-10-02'Versus: jobs:
setup-brioche:
runs-on: ubuntu-latest
steps:
- name: Checkout repository
uses: actions/checkout@v5
- name: Setup Brioche
uses: brioche-dev/setup-brioche@v1
with:
version: 'nightly@2025-10-02' # It could do the trick, if version has a syntax well defined, but what about backward compatibility ? |
|
@jaudiger That's a lot of good points! But I wanted to go a bit more depth on how I was thinking about it more concretely... my take on combining them would be to follow a model pretty similar to Rust's channels + versions (basically the things you can put in the
So that means we'd never need something like Initial support could just cover (1) and (2), meaning the script would check for In the future, I was thinking we could cover all 4 situations generically, so this script wouldn't need to care about channels vs. versions. Instead they would all get routed to a URL like As for the name of the combined field, I personally like how Normally I'd be more lax about just picking one and trying it out, but since |
|
Also, @asheliahut made a good point that it’s pretty common convention for “setup” actions to just use a single field for both purposes: |
Signed-off-by: Jérémy Audiger <[email protected]>
|
@kylewlacy @asheliahut Fair enough, you both convinced me to merge both fields into a single one. I just pushed a commit in this direction, taking in account what you suggested @kylewlacy |
Signed-off-by: Jérémy Audiger <[email protected]>
|
Yeah, sorry I haven't directly said something on this til now, but yeah I definitely looked at how all the big named and supported actions were doing this back when I set out to make this action. I wanted to make sure it was following what most people would be used to in order to facilitate easy adoption. |
Signed-off-by: Jérémy Audiger <[email protected]>
|
Sure, we should follow how things are done usually, I understand. Thanks for the insights @asheliahut. I finally completed this PR, you can have another round of review whenever you want |
kylewlacy
left a comment
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.
LGTM 👍
This should be a big win for our own workflows too!
Once a new release of the GitHub action is available, I’ll make sure to open the PRs to update our workflows 🚀 |
This PR adds the ability to set the Brioche channel to mimic what we can already do with the official Brioche install script (which has been included in this PR: brioche-dev/brioche.dev#49).
Only two values will be accepted:
stablenightlyIf
nightlyis set, it won't be possible to override the Brioche version used, thus an error will be produced if bothversionandchannelfields are present.Bonus point: with this PR, it'll now be possible to use this GitHub action on ARM GitHub runner when using the nightly channel !