Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require an @ in front of all commands. #261

Merged
merged 3 commits into from
Apr 19, 2023
Merged

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 19, 2023

This paves the way for actually implementing rust-lang/compiler-team#512 and thus checking that the commands actually parse and aren't silently ignored.

It also is an intermediate step in rust-lang/rust-clippy#10426 for moving to the ui_test crate

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 19, 2023

I could put the @ requirement behind a crate feature or a runtime config value if preferred.

@Manishearth
Copy link
Owner

Can we not require it yet?

Behind a config is preferred if we are hoping to require it.

@oli-obk
Copy link
Contributor Author

oli-obk commented Apr 19, 2023

Done, this PR should not affect any users, except those who exhaustively construct/destruct Config.

@Manishearth
Copy link
Owner

Done, this PR should not affect any users, except those who exhaustively construct/destruct Config.

I guess that still technically counts as a breaking change. But clippy can update to a git rev temporarily.

@Munksgaard
Copy link
Collaborator

The failing test is unrelated to this change. Will you release a new version, @Manishearth?

@Munksgaard Munksgaard merged commit b43a394 into Manishearth:master Apr 19, 2023
@Manishearth
Copy link
Owner

Needs to be a major bump yeah? Will do.

@Manishearth
Copy link
Owner

Published v0.10.0

Hopefully one of the last publishes of this crate since I'd like to point people towards uitest over time.

@Munksgaard
Copy link
Collaborator

Munksgaard commented Apr 19, 2023

Yeah. Perhaps we should consider deprecating/archiving and pointing to uitest in the README at some point.

@oli-obk oli-obk deleted the @ branch April 19, 2023 21:03
bors added a commit to rust-lang/rust-clippy that referenced this pull request Apr 20, 2023
Update to a compiletest-rs version that requires `//@` for commands

Requires Manishearth/compiletest-rs#261 to get published

This PR is a smaller step towards #10426

changelog: Move to a version of compiletest-rs that allows us to require `//`@`` for test suite commands.
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.

3 participants