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

refactor: update bench #3739

Merged
merged 1 commit into from
Aug 29, 2024
Merged

refactor: update bench #3739

merged 1 commit into from
Aug 29, 2024

Conversation

Conaclos
Copy link
Member

@Conaclos Conaclos commented Aug 29, 2024

Summary

The benchmark was broken.
This PR fixes it and improves the benchmark in several ways:

  • We now have a CLI that allows us to choose a repository and a subset of benchmarks.
    As CLI parser, I picked commander.js because it is battle-tested and has no dependencies.
  • I updated the tool's config and added two new benchmarks: ts-eslint and ts-biome that adds Typescript ESLint rules (without rules that require type info).
  • Update the docs
  • Update the dprint's TypeScript plugin

Note: I disabled parallel-prettier because it crashes on Node 22.

Fixes #2100

Test Plan

I tested manually the new script on Linux.

@Conaclos Conaclos requested review from a team August 29, 2024 09:36
@ematipico
Copy link
Member

Can't we use parseArgs from node:utils as CLI parsers?

Copy link
Member

@SuperchupuDev SuperchupuDev left a comment

Choose a reason for hiding this comment

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

just a npm leftover, you also need to remove the old package-lock.json

Dockerfile.benchmark Show resolved Hide resolved
@SuperchupuDev
Copy link
Member

does this close #2100?

@Conaclos Conaclos force-pushed the conaclos/update-benchmark branch 2 times, most recently from f730d59 to b37b46e Compare August 29, 2024 10:18
@Conaclos
Copy link
Member Author

does this close #2100?

Yes. Thanks for the reminder!

benchmark/package.json Outdated Show resolved Hide resolved
benchmark/eslint.config.js Outdated Show resolved Hide resolved
benchmark/biome.json Outdated Show resolved Hide resolved
benchmark/bench.js Outdated Show resolved Hide resolved
benchmark/bench.js Outdated Show resolved Hide resolved
benchmark/bench.js Outdated Show resolved Hide resolved
benchmark/bench.js Show resolved Hide resolved
@Conaclos Conaclos force-pushed the conaclos/update-benchmark branch 3 times, most recently from 0199dbd to 910eca3 Compare August 29, 2024 10:37
@Conaclos
Copy link
Member Author

Can't we use parseArgs from node:utils as CLI parsers?

Good suggestion. Done :)

@Conaclos Conaclos force-pushed the conaclos/update-benchmark branch 4 times, most recently from 2b4fa8c to 42f7e86 Compare August 29, 2024 10:52
benchmark/bench.js Outdated Show resolved Hide resolved
@Conaclos Conaclos force-pushed the conaclos/update-benchmark branch 2 times, most recently from 9ab46a7 to 1b408cc Compare August 29, 2024 15:44
@Conaclos Conaclos merged commit b03b8dc into main Aug 29, 2024
5 checks passed
@Conaclos Conaclos deleted the conaclos/update-benchmark branch August 29, 2024 15:44
@zuzjes
Copy link

zuzjes commented Sep 19, 2024

There is a typo in benchmark/.prettierignore , to my knowledge there is nothing like htnl. **/*.htnl

@Conaclos
Copy link
Member Author

There is a typo in benchmark/.prettierignore , to my knowledge there is nothing like htnl. **/*.htnl

PR to fix the typo are welcome :)

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.

📎 Benchmark directory improvements
5 participants