Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request addresses issue #198 by renaming configuration variables to remove the "Default" prefix, adds a new "Most Compatible" template, and updates the Option ROM template with a notice regarding its production suitability.
- Renames sections from DefaultPk/DefaultKek/DefaultDb/DefaultDbx to PK/KEK/DB/DBX in multiple template files
- Introduces a new "Most Compatible – Microsoft and 3rd Party" template
- Updates the Option ROM template with a production recommendation notice and adjusts the workflow configuration accordingly
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| Templates/MostCompatible.toml | Adds a new template for a most-compatible configuration |
| Templates/MicrosoftAndThirdParty.toml | Renames configuration sections to remove the "Default" prefix and updates help text |
| Templates/MicrosoftAndOptionRoms.toml | Updates the template header notice and renames configuration sections appropriately |
| .github/workflows/prepare-binaries.yml | Updates workflow to refer to the new MicrosoftAndThirdParty.toml template |
Files not reviewed (1)
- scripts/windows/InstallSecureBootKeys.ps1: Language not supported
Comments suppressed due to low confidence (2)
Templates/MicrosoftAndOptionRoms.toml:2
- [nitpick] The template header name appears inconsistent with the file name 'MicrosoftAndOptionRoms.toml' and its intended Option ROM usage. Consider revising the template name to better reflect its purpose.
# Template Name: Secure Desktop – Microsoft Only
Templates/MicrosoftAndThirdParty.toml:39
- [nitpick] The help text references 'MicrosoftOnlyDefaults', which is inconsistent with the new naming convention. Updating this description for clarity would improve consistency.
help = "Contains 2023 Microsoft and UEFI third party certificates to verify binaries before execution. More compatible than MicrosoftOnlyDefaults."
apop5
reviewed
Apr 28, 2025
apop5
approved these changes
Apr 28, 2025
Javagedes
approved these changes
May 1, 2025
3cba23f to
937fc42
Compare
apop5
approved these changes
May 1, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Resolves feedback in issue #198.
The renaming will be a breaking change however since the release has not been made yet and includes additional breaking changes - now is the correct time to make this change.
How This Was Tested
Local Machine
Integration Instructions
Renaming the "Default.bin" to ".bin"