Skip to content

style: reorder one re-export #1648 left unsorted on main - #1651

Merged
justinchuby merged 1 commit into
mainfrom
squad/roy-fmt-repair-main-1648b
Aug 21, 2026
Merged

justinchuby merged 1 commit into
mainfrom
squad/roy-fmt-repair-main-1648b

Conversation

@justinchuby

Copy link
Copy Markdown
Owner

cargo fmt --all -- --check fails on an unmodified origin/main (90ddd284e):

crates/onnx-runtime-session/src/lib.rs:38

pub use onnx_runtime_ep_api::DeviceGraphSlot; was added above the existing WorkspaceRequirement re-export rather than in sorted order. One-line swap, pure cargo fmt --all output.

Formatting is a required check, so this blocks every open PR regardless of contents. It surfaced on an unrelated CPU-kernel PR (#1628).

This is the fourth main-is-red repair today

# PR what was red on main source
1 #1640 (not mine) fmt + 3 clippy lints, four required gates at once #1637 / #1641
2 #1642 fmt, two sites #1644
3 #1649 clippy dead_code, native_decode/cuda.rs #1648
4 this fmt, one re-export #1647 / #1648

The pattern is consistent and worth fixing at the source: quality gates run on PR branches before merge but not on the merge result, so any merge can land violations that then fail whoever opens the next PR. Because the gates are sequential — clippy steps only run once formatting passes — each breakage costs a full CI round-trip to even discover, and they arrive one at a time.

Two concrete options: enable a merge queue (gates run on the merge result), or run the Rust quality job on main post-merge so the break is attributed to the PR that caused it instead of the next unrelated one.

Also still red on main and not fixed here, because I cannot reproduce it locally and it is not mine: Rust (Windows ARM64) → Test cross-platform offline crates has been failing on main since at least 73e6fe15a (it is non-required, so it does not block merges).

`cargo fmt --all -- --check` fails on an unmodified `origin/main` (90ddd28):

    crates/onnx-runtime-session/src/lib.rs:38

`pub use onnx_runtime_ep_api::DeviceGraphSlot;` was added above the existing
`WorkspaceRequirement` re-export instead of in sorted order. One-line swap,
pure `cargo fmt --all` output.

Formatting is a required check, so this blocks every open PR.

This is the fourth main-is-red repair today (#1640, #1642, #1649, this one).
The pattern is consistent: quality gates are being evaluated on PR branches
before merge but not on the merge result, so each merge can and does land
violations that then fail whoever opens the next PR.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.64%. Comparing base (90ddd28) to head (41ec1a4).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1651      +/-   ##
==========================================
+ Coverage   80.84%   81.64%   +0.80%     
==========================================
  Files         383      382       -1     
  Lines      175886   177266    +1380     
  Branches   175886   177266    +1380     
==========================================
+ Hits       142199   144735    +2536     
+ Misses      28786    27596    -1190     
- Partials     4901     4935      +34     
Flag Coverage Δ
cli-ort-linux ?
cli-ort-windows 82.11% <ø> (-0.10%) ⬇️
mlas ?
offline 81.63% <ø> (+0.92%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
crates/onnx-runtime-session/src/lib.rs 60.65% <ø> (ø)

... and 42 files with indirect coverage changes

🚀 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.

@justinchuby
justinchuby merged commit c729613 into main Aug 21, 2026
14 of 17 checks passed
@justinchuby
justinchuby deleted the squad/roy-fmt-repair-main-1648b branch August 21, 2026 11:20
justinchuby added a commit that referenced this pull request Aug 21, 2026
`Rust quality → Check formatting` is failing on `main` at `6923a016b`
(#1652). That job runs its steps sequentially, so while formatting is
red **every other check in it is skipped** — clippy, publish order, the
dispatch-manifest lints, feature-gate coverage, all of it. Every open PR
is blocked and none of them are getting linted.

Three sites, all in
`crates/onnx-genai-engine/src/native_decode/cuda.rs`: two
`verify_graph_phase` assignments (`:1815`, `:1823`) and one `assert_eq!`
in `verify_capture_helper_tests` (`:7042`). Straight `cargo fmt --all`
output, no hand edits.

**Verified inert.** The before/after texts are identical after stripping
whitespace *and* trailing commas — the only non-whitespace delta is
commas rustfmt adds before a closing delimiter when it breaks a call
across lines, which are semantically meaningless in Rust. Reproduced on
a pristine `origin/main` worktree first, so this is main's breakage and
not an artifact of my branch.

### This is the fifth time today

`main` has been red on formatting or clippy five separate times in one
day: #1637/#1641 (fixed by #1640), #1644 (#1642), #1648 (#1649),
#1647/#1648 (#1651), and now #1652.

The cause is structural, not carelessness. Required checks run on a PR's
**merge ref**, but nothing re-runs them on `main` **after** the merge,
so two PRs that are each green against an older base can land in
sequence and leave the result red. Because the quality job is
sequential, the breakage also masks every later step in it. The cost
lands on whoever opens the next PR, who then has to distinguish "my
change broke this" from "main was already broken" — a full CI round-trip
each time.

Two things would fix it, either one sufficient:
- a **merge queue**, which tests the actual post-merge result; or
- running **`Rust quality` on `main` post-merge**, which at least
detects it immediately and attributes it correctly.

I have now spent four PRs on this. I would rather not spend a fifth. cc
@justinchuby

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

1 participant