-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
Implement template literals and tagged templates #997
Conversation
Codecov Report
@@ Coverage Diff @@
## master #997 +/- ##
==========================================
+ Coverage 58.37% 58.47% +0.10%
==========================================
Files 173 176 +3
Lines 12050 12225 +175
==========================================
+ Hits 7034 7149 +115
- Misses 5016 5076 +60
Continue to review full report at Codecov.
|
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.
Thank you for the PR!
I have some comments for my understanding mostly, and I have some files left to review. Things are looking very good from a conformance point of view:
Test262 conformance changes:
Test result | master count | PR count | difference |
---|---|---|---|
Total | 78,497 | 78,497 | 0 |
Passed | 24,548 | 24,698 | +150 |
Ignored | 15,585 | 15,587 | +2 |
Failed | 38,364 | 38,212 | -152 |
Panics | 28 | 30 | +2 |
Conformance | 31.27 | 31.46 | +0.19% |
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 had a bit of time to check more files, looking good. I had some comments, though.
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.
Numbers look great :) there might be a small performance penalty, but this is expected in any new features.
I have reviewed the code, and I didn't see any potential improvement. I would still like for @Lan2u and/or others to review it.
From my side, it can be merged as-is.
@tofpie please don't merge |
OK. Sorry for that. |
This Pull Request closes #996.
It changes the following: