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

optimize(Path2D): replace reassign-concat with for-push #76

Merged
merged 1 commit into from Feb 24, 2021

Conversation

milahu
Copy link
Contributor

@milahu milahu commented Dec 21, 2020

No description provided.

@coveralls
Copy link

coveralls commented Dec 21, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling f020c9f on milahu:patch-2 into c4d1fb2 on hustcc:master.

@jtenner
Copy link
Collaborator

jtenner commented Dec 21, 2020

This might be a valid performance gain, with the trade-off that the software becomes less simple. Also, why did code coverage decrease?

@jtenner
Copy link
Collaborator

jtenner commented Dec 21, 2020

@milahu I think this might be another micro-optimization, however, it's probably fine.

https://coveralls.io/builds/35861728/source?filename=src%2Fclasses%2FPath2D.js#L43

Please address this code coverage issue if you can. We need to add a way to validate that push gets called.

@milahu
Copy link
Contributor Author

milahu commented Dec 21, 2020

sorry .. note to self: run tests offline

ps: isnt it beautiful? __snapshots__/just-a-start.test.js.actual.js

@hustcc hustcc requested a review from jtenner February 7, 2021 02:30
@hustcc hustcc merged commit 315d56b into hustcc:master Feb 24, 2021
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.

None yet

4 participants