Skip to content
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

Rename long and short #53

Merged
merged 2 commits into from
Sep 3, 2016
Merged

Rename long and short #53

merged 2 commits into from
Sep 3, 2016

Conversation

gobwas
Copy link
Contributor

@gobwas gobwas commented Sep 22, 2015

Cause it is reserved words in ES1-3. It is ok for ES2015, but it still breaks down Closure Compiler.

Cause it is reserved words in ES1-3. It is ok for ES2015, but it still breaks down Closure Compiler.
@@ -25,8 +25,8 @@ module.exports = function(val, options){
options = options || {};
if ('string' == typeof val) return parse(val);
return options.long

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use options['long'], since the reserved word problem can still affect object properties. Also, my syntax checker complained about these not being on the same line, so maybe

return options['long'] ? fmtLong(val) : fmtShort(val);

would be better.

@gobwas
Copy link
Contributor Author

gobwas commented Sep 30, 2015

Yeah, you are right. Fixed it. 😎

@A
Copy link

A commented Oct 7, 2015

@gobwas Good catch! +1

@gobwas
Copy link
Contributor Author

gobwas commented Oct 8, 2015

@shuvalov-anton 😎

@leo
Copy link
Contributor

leo commented Sep 3, 2016

Thanks! ❤️

@leo leo added the enhancement label Sep 3, 2016
@leo leo merged commit 899d463 into vercel:master Sep 3, 2016
@DevSide
Copy link

DevSide commented Oct 24, 2016

Could you publish a tag as quick as possible with this PR ? Thank you

@leo
Copy link
Contributor

leo commented Oct 25, 2016

@DevSide Done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants