-
Notifications
You must be signed in to change notification settings - Fork 310
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
Bootloader probing and construction refactoring #2224
Bootloader probing and construction refactoring #2224
Conversation
I think this may affect bindings too.
In preparation for enhancing `_ostree_sysroot_query_bootloader`
It's easier to extend and it centralises the config parsing. In other places we will no longer need to use `g_str_equal` to match these values, a `switch` statement will be sufficient.
...with the `sysroot.bootloader` configuration option. This can be useful when converting a system to use `ostree` which doesn't currently have a bootloader configuration that `ostree` can automatically detect, and is also useful in combination with the `--sysroot` option when provisioning a rootfs for systems other than the one you're running `ostree admin deploy` on.
This is more regular, so will make it easier to add more bootloader types in the future.
I've made this use functions to make it easier to add support for more bootloaders. Seeing as there will be a big diff anyway I've also adjusted the formatting to make it pep8 compliant.
@lucab: Thanks for the review. |
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
@lucab: changing LGTM is restricted to collaborators In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Hmm, not sure why @lucab's lgtm was rejected. You're in the list of reviewers and your a member of the ostreedev organisation. @cgwalters: any idea? |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lucab, wmanley 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 |
My organization membership was not public, thus the bot could not detect it. Updated my orga/profile settings, it seems to be fine now. |
Thanks :) |
Oh bother: The fixup!s got merged too. I was under the impression that the bot took care of rebasing. Maybe that was the old bot. I will be more careful in the future. |
These are some refactorings I made a little while ago in preparation for adding a new bootloader class, but the new bootloader wasn't necessary after all. I think the refactorings still have value though.
I'm raising this now so it can be used in the
OstreeBootloaderRpi
implementation. See discussion on #2223.