Skip to content

Fix main: replace jsg::Number with f64 in jsg::Dict tests - #8071

Merged
bartlomieju merged 3 commits into
mainfrom
biwanczuk/fix-dict-after-v8-mirror
Oct 28, 2026
Merged

bartlomieju merged 3 commits into
mainfrom
biwanczuk/fix-dict-after-v8-mirror

Conversation

@bartlomieju

Copy link
Copy Markdown
Collaborator

Fixing main because of conflict between #8054 and #7998.

#7998 added uses of jsg::Number, which #8054 removed in favor of f64.
@bartlomieju
bartlomieju requested review from a team as code owners October 28, 2026 17:40
@clanky-the-clanker

clanky-the-clanker Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

✅ Approved — This is a mechanical swap from jsg::Number to f64, plus a justified lint expectation for exact comparisons of small integer-valued floats. No issues were found.

The decision is advisory: this comment never approves, blocks or requests changes on the pull request.


✅ All checks passed

📋 Code Quality: ✅ LGTM

The latest commit adds #[expect(clippy::float_cmp, reason = "...")] to get_by_name_and_keys in src/rust/jsg-test/tests/dict.rs, because the test now compares f64 values exactly. Other tests in the repo handle the lint the same way, for example src/rust/jsg-test/tests/promise.rs and src/rust/jsg-test/tests/number.rs. The reason string is accurate: the compared values, 3.0 and 2.0, are small integers that f64 represents exactly.

The rest of the diff is a mechanical replacement of jsg::Number with f64. f64 provides the Type, ToJS and FromJS implementations that every Dict<...f64...> in the tests needs.


📝 Documentation: ✅ LGTM

The Dict doc example in src/rust/jsg/dict.rs now uses f64 and matches the current API.


Reviewed commit: 7b1830a4 — same changes as 5f159595, so this is that commit's review (no new review was run; line numbers are as of 5f159595)

@ask-bonk

ask-bonk Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Since last review: 0 resolved, 0 still open, 0 new.
LGTM!

Not re-run: tests, api-compat, docs, compat-flags, design-simplicity, jsg-gc, memory-safety, rust-first (no author changes in their files since the last review)


Reviewed commit: 7b1830a4 · github run

@bartlomieju
bartlomieju enabled auto-merge (squash) October 28, 2026 17:44
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

The generated output of @cloudflare/workers-types matches the snapshot in types/generated-snapshot 🎉

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.42%. Comparing base (d68580d) to head (7b1830a).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8071      +/-   ##
==========================================
- Coverage   69.49%   69.42%   -0.07%     
==========================================
  Files         695      696       +1     
  Lines      181564   181742     +178     
  Branches    29204    29211       +7     
==========================================
+ Hits       126170   126180      +10     
- Misses      38328    38496     +168     
  Partials    17066    17066              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bartlomieju
bartlomieju merged commit 2107fd3 into main Oct 28, 2026
26 of 27 checks passed
@bartlomieju
bartlomieju deleted the biwanczuk/fix-dict-after-v8-mirror branch October 28, 2026 19:41
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.

4 participants