-
Notifications
You must be signed in to change notification settings - Fork 5
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
Switch to main and release branches #145
Conversation
build
Outdated
@@ -3,20 +3,14 @@ set -euo pipefail | |||
|
|||
TARGET="${1:-default}" | |||
|
|||
scripts/info-branch-commit | |||
export readonly TARGET=$(scripts/option-prompt "Select a target" vm develop validation master lab-key shed-key stuck default) |
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.
Should we rename default
to all
?
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.
all
is quite good, but maybe not as precise to default
, as it picks default values as well. Do you think it’s still relevant with the last change reverting to previous usage?
build
Outdated
@@ -3,20 +3,14 @@ set -euo pipefail | |||
|
|||
TARGET="${1:-default}" | |||
|
|||
scripts/info-branch-commit | |||
export readonly TARGET=$(scripts/option-prompt "Select a target" vm develop validation master lab-key shed-key stuck default) |
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.
I am not necessarily against an interactive script, but I wouldn't want that to be the only option, because
- I want to be able to script things,
- I want to be able to run a combined command like
./build vm; result/bin/run-in-vm
from history without having to type extra stuff into the terminal.
It would be fine for ./build
to prompt a target if none is given, I think?
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.
I agree, this would not be convenient for me as well, reverting to previous usage in 9048ca6.
Adding info & confirm safeguards for develop
, validation
and master
builds.
Only add info & confirmation steps for builds on: - develop, - validation, - release.
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.
Good for me now 👍
This permits to simplify branch management.