-
Notifications
You must be signed in to change notification settings - Fork 650
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
[sonic_installer] Refactor sonic_installer code #953
[sonic_installer] Refactor sonic_installer code #953
Conversation
Thanks for this enhancement, Samuel! Would you mind pulling the new classes (Bootloader, AbootBootloader, GrubBootloader, UbootBootloader) out into their own sepearate files? This will help make main.py more readable/managable and keep the different bootloader logic isolated. |
Will do! Do you have a preference for the location of these files? I'm thinking of
Alternatively
Let me know what you think. |
@Staphylo: I prefer the bootloader/ subdirectory in the latter, but I believe the base Bootloader class definition should not live in the init.py file, but rather a separate file, like bootloader/bootloader.py or bootloader/bootloader_base.py because the primary purpose of the init.py file is to initialize the package, not to hold content. Thanks! |
42d2882
to
5c831c3
Compare
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.
As comments
13a6b14
to
1f0429f
Compare
Retest this please |
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.
Unblock
Add a new Bootloader abstraction. This makes it easier to add bootloader specific behavior while keeping the main logic identical. It is also a step that will ease the introduction of secureboot which relies on bootloader specific behaviors. Shuffle code around to get rid of the hacky if/else all over the place. There are now 3 bootloader classes - AbootBootloader - GrubBootloader - UbootBootloader There was almost no logic change in any of the implementations. Only the AbootBootloader saw some small improvements. More will follow in subsequent changes.
1f0429f
to
16b5195
Compare
Add a new Bootloader abstraction. This makes it easier to add bootloader specific behavior while keeping the main logic identical. It is also a step that will ease the introduction of secureboot which relies on bootloader specific behaviors. Shuffle code around to get rid of the hacky if/else all over the place. There are now 3 bootloader classes - AbootBootloader - GrubBootloader - UbootBootloader There was almost no logic change in any of the implementations. Only the AbootBootloader saw some small improvements. More will follow in subsequent changes.
The import re line was removed from the file in #953. However, it is still referenced in the update_sonic_environment() function. Adding it back.
Add a new Bootloader abstraction.
This makes it easier to add bootloader specific behavior while keeping
the main logic identical.
It is also a step that will ease the introduction of secureboot which
relies on bootloader specific behaviors.
Shuffle code around to get rid of the hacky if/else all over the place.
There are now 3 bootloader classes
There was almost no logic change in any of the implementations.
Only the AbootBootloader saw some small improvements.
More will follow in subsequent changes.