-
Notifications
You must be signed in to change notification settings - Fork 166
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
Add an image-default.yaml, pass JSON to create_disk.sh #1787
Add an image-default.yaml, pass JSON to create_disk.sh #1787
Conversation
This is prep work for a larger refactoring for coreos/bootupd#50 |
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.
Looks like a nice cleanup to me! I like also that we have all the defaults in a separate YAML file.
I can /lgtm this if you'd prefer to convert the other ones in a separate PR. |
The way we parse an `image.yaml` and then convert it into arguments for `create_disk.sh` is unwieldy. Instead, convert `image.yaml` into JSON, inheriting default values from a separate `image-default.yaml` we ship. Then pass that JSON to `create_disk.sh`. This only converts the `rootfs` argument, but if people are OK with this I can go ahead and convert everything else - getting us to where the only argument to `create_disk.sh` is a JSON file that's already been pre-validated.
f49aa24
to
b769f56
Compare
OK rebased 🏄 |
@@ -92,6 +92,17 @@ remote_name="${remote_name:?--ostree-remote must be defined}" | |||
grub_script="${grub_script:?--grub-script must be defined}" | |||
os_name="${os_name:?--os_name must be defined}" | |||
|
|||
getconfig() { | |||
k=$1 | |||
jq -re .$k < ${config} |
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: could just drop <
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The way we parse an
image.yaml
and then convert itinto arguments for
create_disk.sh
is unwieldy.Instead, convert
image.yaml
into JSON, inheriting default valuesfrom a separate
image-default.yaml
we ship. Then passthat JSON to
create_disk.sh
.This only converts the
rootfs
argument, but if peopleare OK with this I can go ahead and convert everything
else - getting us to where the only argument to
create_disk.sh
is a JSON file that's already been pre-validated.