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 #7398: Update compliance UI to manage disable mode #963
Fixes #7398: Update compliance UI to manage disable mode #963
Conversation
9c61301
to
9a41658
Compare
@@ -304,7 +304,7 @@ case object LongComparator extends CriterionType { | |||
|
|||
case object MemoryComparator extends CriterionType { | |||
override val comparators = OrderedComparators.comparators | |||
override protected def validateSubCase(v:String,comparator:CriterionComparator) = try { | |||
override protected def validateSubCase(v:String,comparator:CriterionComparator) = { |
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.
shouldn't you keep the try ?
What happens if the parse fails ?
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.
there is no catch
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.
then why not adding a catch ??
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.
because it is not supposed to throw exception (Full/Failure), the try must be a remeniscience of the past
9a41658
to
02262d4
Compare
PR updated |
|
||
val complianceWithoutPending = pc_for(success+repaired+notApplicable, total-pending) | ||
val complianceWithoutPending = pc_for(success+repaired+notApplicable, total-pending-reportsDisabled) |
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.
Ha, shouldn't you remove the reportsDisabled on the left side as well ?
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.
ha ok
Thank you @fanf |
…nage_disable_mode Fixes #7398: Update compliance UI to manage disable mode
https://www.rudder-project.org/redmine/issues/7398