-
Notifications
You must be signed in to change notification settings - Fork 74
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 #8782: Fix details on script (remove $ from token and fix double password prefix) #1153
Conversation
…e password prefix)
Commit modified |
3543bc5
to
1500591
Compare
final object DisabledEngine extends JsEngine { | ||
/* | ||
* Eval does nothing on variable without the EVAL keyword, and | ||
* fails on variable with the keyword. | ||
*/ | ||
def eval(variable: Variable, lib: JsRudderLibBinding): Box[Variable] = { | ||
val (default, js, _) = getEvaluatorTuple(variable) | ||
variable.values.find( x => x.startsWith(default) || x.startsWith(js) ) match { | ||
case None => Full(variable) | ||
variable.values.find( x => x.startsWith(default)) match { |
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 think it's less readable to have two separated tests. Or maybe add a comment before (i guess it's for performance reason), and move the comme "Here, we nned to chose" just before this line
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 find it less readable too, but it was not able to differentiate which EVAL was found,
Séparate them allow a much more précise error message
Le 29 juillet 2016 07:53:43 GMT+02:00, Nicolas Charles notifications@github.com a écrit :
final object DisabledEngine extends JsEngine {
/*
* Eval does nothing on variable without the EVAL keyword, and
* fails on variable with the keyword.
*/
def eval(variable: Variable, lib: JsRudderLibBinding):
Box[Variable] = {
val (default, js, _) = getEvaluatorTuple(variable)
x.startsWith(js) ) match {variable.values.find( x => x.startsWith(default) ||
case None => Full(variable)
variable.values.find( x => x.startsWith(default)) match {
i think it's less readable to have two separated tests. Or maybe add a
comment before (i guess it's for performance reason), and move the
comme "Here, we nned to chose" just before this line
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub:
https://github.com/Normation/rudder/pull/1153/files/150059137b7d15c19d9169ad39ce59aea863c83d#r72744578
Envoyé de mon appareil Android avec K-9 Mail. Veuillez excuser ma brièveté.
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.
having the "Here, we need to chose betwwen" in between masks that they are related and do the same
Appart from the comment part, this lloks great to me ! |
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/8782