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
Cast js-defined variables to string #84
Conversation
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.
Looks good @shonie!
Some small nits
test/test.js
Outdated
'null-value': null, | ||
'undefined-value': undefined, | ||
'object-value-passed-by-mistake': {} | ||
}; |
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.
We should add a Boolean
(true
/false
)
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.
Agree, done in shonie@2c651e8f3df1b835aa023375d74a2306c9bfef96
test/test.js
Outdated
@@ -22,6 +22,14 @@ var MOCK_JS_VARIABLES = { | |||
'js-defined-no-prefix': '#ff0000' | |||
}; | |||
|
|||
var INVALID_JS_VARIABLES = { |
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.
Perhaps rename to NON_STRING_VARIABLES
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.
Agree too. It is in shonie@2c651e8f3df1b835aa023375d74a2306c9bfef96 too.
Can i squash commits now?
Thank you for review!
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.
Squash welcome 🙂
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.
Done!
PR #84 remarks
Published in Thanks @shonie ❤️ |
You welcome, @MadLittleMods! 🙂 |
Fix #83
Cast js-defined variables to string