BG suite multierror support #362
Open
Conversation
This file contains 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
ChriMarMe
requested changes
Jan 29, 2024
pkg/test/bootguard_tests.go
Outdated
Comment on lines
29
to
32
| Name: "SACM meets sane BootGuard requirements", | ||
| Required: true, | ||
| function: BootGuardACM, | ||
| dependencies: []*Test{&testbootguardfit}, |
There was a problem hiding this comment.
I like the alignemet before the change more :)
There was a problem hiding this comment.
this comes from the Go formatter, which my IDE ran automatically... 😅
There was a problem hiding this comment.
I realized that this happened due to the comments. So, I just removed them. :)
orangecms
requested review from
zaolin,
walterchris,
PatrickRudolph and
ArthurHeymans
as code owners
|
So I checked because I noticed that go-multierror was already in the dependencies... Which led me to this: Apparently, Go 1.20 added native multi error support. Anyway, I think introducing the Uber package is fine here, and we can rework things nicely again later. |
|
And the CI error basically says that we should upgrade to Go 1.19 at least (I would go straight to 1.21). Huh, we do have 1.19 though... no idea what's wrong here... sigh Edit/addendum: sooooo... since this uses atomics and Go doesn't offer them on all architectures we use, I'll see about using |
orangecms
changed the title
|
❤️ |
ChriMarMe
reviewed
Jan 31, 2024
| @@ -29,7 +30,6 @@ var ( | |||
| Name: "SACM meets sane BootGuard requirements", | |||
| Required: true, | |||
| function: BootGuardACM, | |||
| dependencies: []*Test{&testbootguardfit}, | |||
There was a problem hiding this comment.
- Why remove the dependency?
- Why not in a seperate commit?
|
The dependency removal is the actual feature here; this before vs after should clarify it: beforeafter |
|
grammar fixed: |
orangecms
force-pushed
the
multierr
branch
4 times, most recently
from
24e5066 to
5c9217d
Compare
Signed-off-by: Daniel Maslowski <[email protected]>
Signed-off-by: Daniel Maslowski <[email protected]>
This uses go.uber.org/multierr. Also reword many error messages to make them consistent: - capitalize BootGuard, Key Manifest and Boot Policy Manifest - spell out Key Manifest and Boot Policy Manifest - uppercase acronyms such as DMA etc Signed-off-by: Daniel Maslowski <[email protected]>
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.
Signed-off-by: Daniel Maslowski [email protected]