Open
Conversation
ChriMarMe
requested changes
Jan 29, 2024
Collaborator
ChriMarMe
left a comment
ChriMarMe
left a comment
There was a problem hiding this comment.
I like the idea of multierror.
Comment on lines
29
to
32
| Name: "SACM meets sane BootGuard requirements", | ||
| Required: true, | ||
| function: BootGuardACM, | ||
| dependencies: []*Test{&testbootguardfit}, |
Collaborator
There was a problem hiding this comment.
I like the alignemet before the change more :)
Contributor
Author
There was a problem hiding this comment.
this comes from the Go formatter, which my IDE ran automatically... 😅
Contributor
Author
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
ArthurHeymans,
PatrickRudolph,
walterchris and
zaolin
as code owners
Contributor
Author
|
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. |
Contributor
Author
|
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
Collaborator
|
❤️ |
ChriMarMe
reviewed
Jan 31, 2024
| Name: "SACM meets sane BootGuard requirements", | ||
| Required: true, | ||
| function: BootGuardACM, | ||
| dependencies: []*Test{&testbootguardfit}, |
Collaborator
There was a problem hiding this comment.
- Why remove the dependency?
- Why not in a seperate commit?
Contributor
Author
|
The dependency removal is the actual feature here; this before vs after should clarify it: beforeafter |
Contributor
Author
|
grammar fixed: |
orangecms
force-pushed
the
multierr
branch
5 times, most recently
from
5c9217d to
41cc2f4
Compare
Signed-off-by: Daniel Maslowski <info@orangecms.org>
Signed-off-by: Daniel Maslowski <info@orangecms.org>
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 <info@orangecms.org>
walterchris
force-pushed
the
multierr
branch
from
41cc2f4 to
fe57f4c
Compare
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
2 participants
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 info@orangecms.org