-
Notifications
You must be signed in to change notification settings - Fork 38
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 #20884: package state options method breaks "Packages" techniques with new reporting #1341
Conversation
Commit modified |
8690e4e
to
126538b
Compare
"args" slist => { "${name}", "${version}", "${architecture}", "${provider}", "${state}", "${options}" }; | ||
"report_param" string => join("_", args); | ||
"full_class_prefix" string => canonify("package_state_options_${report_param}"); | ||
"old_class_prefix" string => string_head("${full_class_prefix}", "1000"); |
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 is really confusing to set "old_class_prefix" to the previous class_previous while "class_prefix" is the "old_class_prefix"
Can you use "previous_class_prefix" ?
Commit modified |
126538b
to
5899af1
Compare
Commit modified |
5899af1
to
6ce392c
Compare
tree/20_cfe_basics/common.cf
Outdated
@@ -114,6 +114,28 @@ body classes classes_generic_two(x,y) | |||
}; | |||
} | |||
|
|||
body classes cancel_classes(y) | |||
{ | |||
cancel_repaired => { "promise_repaired_$(y)", "$(y)_repaired", "$(y)_ok", "$(y)_reached", "$(y)_not_kept" |
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 are dusplicates in this list
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.
plus we already have a tree/30_generic_methods/_classes_cancel.cf method which should do the job
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.
Commit modified |
6ce392c
to
09f5109
Compare
Commit modified |
09f5109
to
a579509
Compare
Commit modified |
a579509
to
9f9e85e
Compare
Commit modified |
9f9e85e
to
41ede36
Compare
…ques with new reporting
OK, merging this PR |
Commit modified |
41ede36
to
b680d71
Compare
Merged with ugly list |
https://issues.rudder.io/issues/20884