Skip to content

Fix let_underscore_drop lint error in dice_tracker - #1406

Closed
daandemeyer wants to merge 1 commit into
facebook:mainfrom
daandemeyer:fix-let-underscore-drop
Closed

Fix let_underscore_drop lint error in dice_tracker#1406
daandemeyer wants to merge 1 commit into
facebook:mainfrom
daandemeyer:fix-let-underscore-drop

Conversation

@daandemeyer

Copy link
Copy Markdown
Contributor

OSS CI (test.py / cargo clippy) is currently failing on every PR and on main itself with:

error: non-binding let on a type that has a destructor
   --> app/buck2_server/src/dice_tracker.rs:138:9

0578509 added the HydrationFailed variant with a String field to DiceEvent, giving it a destructor. That armed the deny-listed let_underscore_drop lint (lint_levels.bzl) on the pre-existing let _ = in BuckDiceTracker::event. Bind to _ignored instead, matching other call sites that discard unbounded_send results.

🤖 Generated with Claude Code

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 24, 2026
@meta-codesync

meta-codesync Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

This pull request has been imported. If you are a Meta employee, you can view this in D113539998. (Because this pull request was imported automatically, there will not be any future comments.)

The new HydrationFailed variant added a String field to DiceEvent, giving
it a destructor. That arms the deny-listed let_underscore_drop lint on the
pre-existing `let _ =` in BuckDiceTracker::event, and the accompanying
paging test added another violation, breaking `cargo clippy` in OSS CI
for every PR. Bind to `_ignored` like other call sites that discard such
results.

Signed-off-by: Daan De Meyer <daan@amutable.com>
@daandemeyer
daandemeyer force-pushed the fix-let-underscore-drop branch from 529a277 to 22844ba Compare July 24, 2026 10:43

@ndmitchell ndmitchell left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review automatically exported from Phabricator review in Meta.

@daandemeyer

Copy link
Copy Markdown
Contributor Author

The macos failure here is an unrelated timing flake: epoch::worker::tests::test_cancellation_sync_correct_context_does_not_cache panicked with elapsed_ms=650 but expected < 650 — exactly on the boundary on a slow runner. The same test passed on linux and windows in this run, and the identical two commits went green on all three platforms (macos included) in the CI run for #1364, which stacks on top of this fix.

@meta-codesync meta-codesync Bot closed this in 3c9f680 Jul 24, 2026
@meta-codesync

meta-codesync Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in 3c9f680.

@meta-codesync meta-codesync Bot added the Merged label Jul 24, 2026
@ndmitchell

Copy link
Copy Markdown
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants