Repository navigation
Conversation
Zious11
commented
Oct 5, 2026
Zious11
left a comment
Owner
Author
There was a problem hiding this comment.
PR Review: #478 (fix PR: clippy 1.99 clone_on_copy)
Verdict: APPROVE (posted as a comment because the PR is self-authored and GitHub does not allow approving your own PR)
covered_sha: 93b10804de05a412b8ca3a471e9ce5069bedd596
What I checked
- The diff is what the description says. One file (
tests/reporter_terminal_tests.rs), +2/-2, one commit. The only changes areClone::clone(&a)->aat L4002 (mod story_120) andClone::clone(&grouped_collapsed)->grouped_collapsedat L4332 (mod story_122). Nothing else changed. FindingsRenderisCopy. At the head SHA,src/reporter/terminal.rs:143has#[derive(Debug, Clone, Copy, PartialEq, Eq)]onpub struct FindingsRender. That is why clippy 1.99 now flags the UFCSClone::clone(&x)form asclone_on_copy.- The tests behave the same. For a
#[derive(Clone, Copy)]type, the derivedclone()returns*self, which is a bitwise copy, the same aslet c = a. Every assertion compares the same values as before (a == c,grouped_collapsed == gc2). The source ofaandgrouped_collapsedis still usable after the move because they areCopy, so the laterassert_eq!(a, ...)andassert_eq!(grouped_collapsed, ...)lines still compile and pass. - The
Clonederive is still enforced.Cloneis a supertrait ofCopy, so droppingClonefrom the derive would fail to compile even with the explicit call removed. - Production code is untouched. Only
tests/changed, andtests/is excluded from the changelog gate, so no CHANGELOG entry is needed. The CHANGELOG gate is green. - Commit and title follow the conventional format (
test: ...). The Semantic PR check passes. - CI at review time: Format, Semantic PR, CHANGELOG, Deny, Action pin and the other gates pass. Clippy, Test, Audit and Fuzz build were still running. This approval assumes Clippy and Test finish green before merge.
Findings
| # | Severity | Category | Finding | Suggestion |
|---|---|---|---|---|
| 1 | NIT | coverage | test_findings_render_derives_debug_clone_copy_partialeq_eq (L4002) and the story_122 "Clone semantics" block (L4332) no longer call Clone at runtime. The Clone bound is still checked at compile time through the Copy supertrait, so nothing is lost. The assertion messages ("cloned Grouped", "Clone + PartialEq ... == clone") now describe a copy. |
Optional: add fn assert_clone<T: Clone>() {} assert_clone::<FindingsRender>(); to make the Clone check explicit, or reword the messages. Not required for merge. |
No BLOCKING or WARNING findings. Demo evidence, BC traceability and story-spec checks don't apply because this is a toolchain-drift fix PR, not a story.
Zious11
added a commit
that referenced
this pull request
Oct 5, 2026
… drift), fix PR #478 green; awaiting human merges (D-579)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix PR (toolchain drift, not a story). CI's stable toolchain moved to clippy 1.99 (rustc 1.99.0, 2026-09-28), which newly flags
clippy::clone_on_copyunder-D warnings. This blocks every PR, including STORY-188 PR #477 (CI run 37271461059), and develop's next run.Fix
tests/reporter_terminal_tests.rsonly, 2 lines.FindingsRenderisCopy, so:Clone::clone(&a)->a(~line 4002)Clone::clone(&grouped_collapsed)->grouped_collapsed(~line 4332)Verification
--keep-going: these were the only 2 errors in the tree-D warnings: clean after fixcargo fmt --check: cleancargo test --all-targets: 2803 passed / 0 failed (matches develop baseline)Impact
Test-only, no production behavior change. Unblocks #477.