-
Notifications
You must be signed in to change notification settings - Fork 19
efi/preinstall: reordered some preinstall check actions #470
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
base: master
Are you sure you want to change the base?
efi/preinstall: reordered some preinstall check actions #470
Conversation
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.
Pull request overview
This PR reorders action sequences in the preinstall check error handling system to prioritize automatic PPI-based actions over manual firmware settings actions. The PR also corrects multiple spelling and grammatical errors throughout the codebase.
Key changes:
- Automatic TPM actions (via PPI) are now listed before manual reboot-to-firmware-settings actions
- Added documentation explaining the rationale for action ordering
- Fixed numerous spelling errors in comments and error messages
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| efi/preinstall/errors.go | Corrected spelling errors in comments and error messages throughout the file |
| efi/preinstall/checks_context.go | Reordered action sequences to prioritize automatic actions and added documentation explaining the ordering rationale |
| efi/preinstall/actions.go | Fixed typos in action constant documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6f7a2c3 to
d63796c
Compare
13d9d11 to
9140439
Compare
9140439 to
4709c58
Compare
4709c58 to
d5d7d5d
Compare
|
I think this looks ok to me, but I am a bit confused about the user experience. Whilst I understand how ordering the returned errors will be presented (that's something I'm going to work on before Christmas), I'm still a bit unsure how this will be presented - ie, is the intention to only present the first action, and how are the others presented if there is more than one? |
Hi @chrisccoulson, thanks for having a look. The basic UX was captured here: Using the Snapd Error Kinds and Actions APIs. (Requires your review).
|
ZeyadYasser
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, Thank you!
Reordered some error-kinds actions according in accordance with prioritization guideline and use new helper to insert ActionProceed in accordance to the guideline.
This proposal is a result of discussions around action order that happened during review of Using the Snapd Error Kinds and Actions APIs.
It was also suggested that we do not need manual actions at all, given the current manual actions all have automatic PPI based counterparts, however I though to keep them in place for the moment while its still early days, so that we have an additional path to achieve actions.
Also corrected a few spelling mistakes.