-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: cleanup benchmark and (dev)Dependencies #14
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
Conversation
|
WalkthroughThe updates include broadening Node.js version coverage in CI, upgrading development dependencies, removing unused packages, and updating the benchmark implementation to hash objects directly with base64url encoding. Benchmark results in both documentation and output files were refreshed to reflect improved performance metrics. No exported or public API changes were made. Changes
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
benchmark.jsOops! Something went wrong! :( ESLint: 9.30.1 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js Warning Review ran into problems🔥 ProblemsCheck-run timed out after 90 seconds. Some checks/pipelines were still in progress when the timeout was reached. Consider increasing the reviews.tools.github-checks.timeout_ms value in your CodeRabbit configuration to allow more time for checks to complete. 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🧰 Additional context used🧬 Code Graph Analysis (1)benchmark.js (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
🔇 Additional comments (6)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
commit: |
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.
Pull Request Overview
This PR cleans up benchmarking scripts and bumps development dependencies across the codebase.
- Bump various devDependency versions and remove unused packages from package.json.
- Refactor benchmark.js to drop external URL-encoding/flattie libs in favor of built-in
base64url
digests and update benchmark results inbenchmark.txt
andREADME.md
. - Extend CI matrix to include Node 18.
Reviewed Changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
package.json | Updated devDependency versions, removed ab64 and flattie . |
benchmark.js | Removed encodeUrl & flattie imports; use digest('base64url') . |
benchmark.txt | Updated recorded benchmark numbers. |
README.md | Synced benchmark table with latest numbers. |
.github/workflows/ci.yml | Added Node 18 to the test matrix. |
Comments suppressed due to low confidence (1)
benchmark.js:57
- Node.js 18 does not support the 'base64url' encoding in crypto.Hash.digest, causing this script to fail under Node 18. Consider falling back to
.digest('base64')
and then replacing+/
/=
characters for URL-safe output or otherwise polyfilling base64url support.
crypto.createHash('sha512').update(hash(obj)).digest('base64url')
size-limit report 📦
|
Deploy preview for stable-hash-x ready! ✅ Preview Built with commit 7e64b2a. |
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.
Important
Looks good to me! 👍
Reviewed everything up to 7e64b2a in 1 minute and 11 seconds. Click for details.
- Reviewed
191
lines of code in5
files - Skipped
1
files when reviewing. - Skipped posting
7
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .github/workflows/ci.yml:17
- Draft comment:
Added Node.js version 18 in the CI matrix; please confirm that all tests and compatibility are ensured on Node 18. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
2. README.md:187
- Draft comment:
Benchmark table metrics have been updated—verify these numbers are reproducible and reflect expected performance. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
3. benchmark.js:3
- Draft comment:
Removed 'encodeUrl' and 'flattie' imports; ensure that any necessary object flattening or encoding adjustments are now handled internally by the hash implementation. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the author to ensure that the removal of 'encodeUrl' and 'flattie' imports is handled internally by the hash implementation. This is a request for confirmation of intention, which violates the rules. The comment does not provide a specific code suggestion or ask for a specific test to be written.
4. benchmark.js:56
- Draft comment:
Switching to native 'base64url' encoding simplifies the code; please verify that the Node versions in use fully support this encoding option. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
5. benchmark.js:53
- Draft comment:
The JSDoc now specifies the parameter as an 'object' instead of 'unknown'. Confirm that this change is intentional since the library supports hashing of primitives too. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
6. benchmark.txt:4
- Draft comment:
Benchmark output has been refreshed; please ensure these performance improvements are consistent across different environments. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
7. package.json:53
- Draft comment:
DevDependencies have been updated with several version bumps and removals (e.g. 'ab64' and 'flattie'). Please verify that these changes do not adversely affect builds or tests. - Reason this comment was not posted:
Confidence changes required:33%
<= threshold50%
None
Workflow ID: wflow_qFP4M9KbRPepGqqf
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
📊 Package size report -0.25%↓
Unchanged files
🤖 This report was automatically generated by pkg-size-action |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Important
Clean up benchmark by removing unused imports, update hashing functions, add Node.js 18 to CI, and update dependencies.
encodeUrl
andflattie
frombenchmark.js
.benchmark.js
to usebase64url
encoding.ci.yml
.@1stg/common-config
to^14.3.0
,@types/node
to^22.16.0
,@types/web
to^0.0.245
,eslint
to^9.30.1
,prettier
to^3.6.2
,react-router-dom
to^7.6.3
, andvite
to^7.0.2
inpackage.json
.ab64
andflattie
fromdevDependencies
inpackage.json
.This description was created by
for 7e64b2a. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores