Forced rustfmt is a roadblock to contributing

To stick another voice on it, you want ./x.py fmt. This will use the pinned, in-tree version of rustfmt, which is both a) guaranteed to work (as CI is gated on it working) and b) won't disagree with ./x.py tidy, like a local version of rustfmt might.

When working in the rustc tree, the current state is that you have to use ./x.py test (which always runs tidy, even when narrowed) rather than cargo test. Even if you can/do get away with cargo-driven tests rather than bootstrap-driven tests, when in the rustc tree you should get into the habit of using ./x.py tidy.

As for making things better:

Enforced-rustfmt has been a net-positive for the rustc tree, because it eliminated human formatting nits and let the repository with ages of history follow a consistent formatting style.

But I'd agree that making a tidy fail not immediately halt the rest of CI fail would also be a positive. Rather than "fix tidy errors and try again," we could actually point out a useful error (or pass), with an additional note of "fix tidy before we allow merge".

The problem is that this requires some sort of "yellow" state for the PR CI. For smaller repos on GitHub Actions, this is typically done by checking style in one job and tests in another. For a repository at rustc's scale, however, splitting jobs arbitrarily just for partial CI feedback can result in a large impact on server time (thus cost), so I don't know how tenable that would be with the current CI architecture.

15 Likes