Skip to content

Improve tokenstream performance, implement prettier #62

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

Merged
merged 3 commits into from
Feb 4, 2018

Conversation

jacobp100
Copy link
Contributor

@jacobp100 jacobp100 commented Feb 4, 2018

Improves tokenstream performance by removing all calls to [].slice. See my comments at #61. All tests pass.

Sorry, I didn't realise I had turned prettier on until I was mostly through this. Added it as a dev dependency—we should use it here from now on! @mxstbr did you want to change the prettier config to match the rest of the projects?

@mxstbr
Copy link
Member

mxstbr commented Feb 4, 2018

It'd be great to copy the prettier config from the main repo+add lint-staged. We should also add CI to this repo?!

@jacobp100
Copy link
Contributor Author

jacobp100 commented Feb 4, 2018

Is your lint-staged checking the files have been run through prettier? And turns out we added a travis file ages ago, I just never turned it on :P

@jacobp100 jacobp100 requested review from kristerkari and removed request for kristerkari February 4, 2018 19:39
Copy link
Member

@mxstbr mxstbr left a comment

Choose a reason for hiding this comment

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

Since tests pass let's ship this?! Next time it'd be great to separate the actual change from formatting stuff in at least separate commits 😉

Copy link
Contributor

@kristerkari kristerkari left a comment

Choose a reason for hiding this comment

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

A bit hard to review the code changes together with the style changes, but all the tests are still passing so looks good :)

@jacobp100 jacobp100 merged commit 156a4af into master Feb 4, 2018
@jacobp100 jacobp100 deleted the token-stream-perf branch February 4, 2018 21:07
@jacobp100
Copy link
Contributor Author

jacobp100 commented Feb 4, 2018

Sorry about that, I'm so used to it, I didn't realise this project didn't have it already when doing the changes!

Published v2.1.1

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.

3 participants