-
-
Notifications
You must be signed in to change notification settings - Fork 38
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
Add fastest and slowest to benchmark #37
Conversation
woss
commented
Apr 13, 2024
•
edited
Loading
edited
- Add fastest and slowest to benchmarks
- upgrade packages
- improve actions CI workflow
this PR includes the formatting of benchmarks js file and package.json natural sorting |
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.
please restore the old linting
b78a7bb
to
e3e6e01
Compare
@mcollina done |
e3e6e01
to
15ab5d2
Compare
i'm not going to change the benchmarks in the README.md since I have a better machine than what's tested on.
|
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.
lgtm
@mcollina i see that CI is failing. node 15 with standard.js issue. it would be good to have a recommit hook |
- Add fastest and slowest to benchmarks - upgrade packages
15ab5d2
to
368ff0a
Compare
Can you please update gha workflow so that all tests pass? Yopu can copy the logic from: |
@mcollina i've updated the ci.yaml. it seems that nullish coalescing operator is added in 14 + based on https://stackoverflow.com/a/68692190/2764898 I tested |
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.
lgtm
the legacy script is missing. |
0942b16
to
e21ec3c
Compare
Can you remove 0.10, 0.12 and 4, those were never supported here |
e21ec3c
to
a0af68d
Compare
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.
lgtm
@mcollina please don't forget to update the README with the benchmarks ran on your hardware |