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
Fixes #20851: Regroup report with their values, not with the expanded value #4196
Conversation
Commit modified |
5c99638
to
b4c8db7
Compare
Tests are not passing |
@@ -342,6 +342,7 @@ final case class BlockStatusReport ( | |||
} | |||
final case class ValueStatusReport ( | |||
componentName : String | |||
, unexpanded : String |
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.
i'd rather have unexpandedComponentName, otherwise we may confuse it with the unexpandedValue
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.
I'll go with "ExpectedComponentName" with is more clear to me the unexpanded. Same for values
@@ -439,14 +440,14 @@ object ComponentValueStatusReport extends Loggable { | |||
* them by component *unexpanded* value | |||
*/ | |||
def merge(values: Iterable[ComponentValueStatusReport]): List[ ComponentValueStatusReport] = { | |||
val pairs = values.groupBy(_.unexpandedComponentValue).map { case (unexpanded, values) => | |||
val pairs = values.toList /*groupBy(_.unexpandedComponentValue).map { case (unexpanded, values) => |
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.
this seems dubious
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.
I did wanted to no merge them, but i changed my mind and now i want them merged by real value, then by expectedValue
Commit modified |
b4c8db7
to
a67a6c1
Compare
Commit modified |
a67a6c1
to
36dcb52
Compare
Tests should now be ok |
OK, merging this PR |
https://issues.rudder.io/issues/20851