Rewrite policy resolver #1465
Merged
Rewrite policy resolver #1465
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
jaym
force-pushed
the
jdm/rewrite-policy-resolver
branch
from
dc4d538
to
f89203b
Compare
jaym
force-pushed
the
jdm/rewrite-policy-resolver
branch
from
f89203b
to
9b9d5dd
Compare
jaym
force-pushed
the
jdm/rewrite-policy-resolver
branch
2 times, most recently
from
b094599
to
c0c98b1
Compare
imilchev
reviewed
Nov 13, 2024
policy/resolved_policy_builder.go
Outdated
| // to. We need to do this because we cannot have a space frame and space | ||
| // policy reporting job because they would have the same qr id. | ||
| // TODO: we should create a new reporting job type for asset and space | ||
| // reporting jobs so its cleare that we can connect both frameworks and |
There was a problem hiding this comment.
Suggested change
| // reporting jobs so its cleare that we can connect both frameworks and | |
| // reporting jobs so its clear that we can connect both frameworks and |
imilchev
approved these changes
Nov 13, 2024
czunker
reviewed
Nov 13, 2024
| return nil, err | ||
| } | ||
|
|
||
| return resolvedPolicy, nil |
There was a problem hiding this comment.
Definitely not a blocker, more like personal preference:
On first read, I skipped the return statement and was expecting an else.
I'm not sure whether it is actually possible, but would it be possible to choose between current and ng resolver outside of the tryResolve?
As said, more like a personal preference, feel free to ignore it.
|
A bug i've found so far:
|
This will make it impossible to tell what the actual score was. In this case, we will set the impact to 0 only when we merge it into a policy
jaym
force-pushed
the
jdm/rewrite-policy-resolver
branch
from
44eefa1
to
94971c5
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Things that are known to be different from the old implementation: