Old syntax suggestion#13874
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @weihanglo (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
rustbot
added
A-build-scripts
| .contains(&&*prefix) | ||
| .then(|| { | ||
| format!( | ||
| "Consider using the old `cargo:` syntax in front of `{prefix}`.\n" |
There was a problem hiding this comment.
imo splitting up the suggestion like this feels a little more awkward to read.
Maybe
| "Consider using the old `cargo:` syntax in front of `{prefix}`.\n" | |
| "Switch to `cargo:{prefix}` (note the single colon).\n" |
There was a problem hiding this comment.
I wanted to make it clear that the single colon is an old syntax. What do you think?
There was a problem hiding this comment.
Instead of splitting the full instruction to two places, I'd like to see it combined.
There was a problem hiding this comment.
OK, how about now?
| if !new_syntax_added_in.is_compatible_with(msrv.as_partial()) { | ||
| let prefix = format!("{key}="); | ||
|
|
||
| let old_syntax_suggestion = RESERVED_PREFIXES |
There was a problem hiding this comment.
When it isn't in RESERVED_PREFIXES, if the key is metadata, we should suggest cargo:{value}
Unsure if we should say much more otherwise. The key is either not supported on their MSRV or they didn't realize they should use metadata= first with cargo::
torhovland
force-pushed
the
old-syntax-suggestion
branch
from
9686982 to
e4e6261
Compare
|
@rustbot review |
weihanglo
left a comment
weihanglo
left a comment
There was a problem hiding this comment.
Thanks for the work!
It would be great if you could clean up some intermediate commits, and make the git history easier to track in the future.
torhovland
force-pushed
the
old-syntax-suggestion
branch
from
7663e63 to
3ea3638
Compare
|
OK, I squashed a couple commits. Hope that's better. |
|
@bors r+ Thanks! (To me I might also make git commit messages clearer. You can see how people did it in this repo. Let's don't bother on this at this moment and move on 👍🏾) |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
Update cargo 7 commits in 0ca60e940821c311c9b25a6423b59ccdbcea218f..4de0094ac78743d2c8ff682489e35c8a7cafe8e4 2024-05-08 01:54:25 +0000 to 2024-05-09 16:09:22 +0000 - Fix docs for unstable script feature (rust-lang/cargo#13893) - Refactor cargo lint tests (rust-lang/cargo#13880) - test(rustfix): run some tests only on nightly (rust-lang/cargo#13890) - Old syntax suggestion (rust-lang/cargo#13874) - docs: clarify dash replacement rule in target name (rust-lang/cargo#13887) - Add local-only build scripts example in check-cfg docs (rust-lang/cargo#13884) - docs(changelog): also mention `--message-format=json` (rust-lang/cargo#13882) r? ghost
6 participants
Fixes #13868.
The build error in the issue will now include a suggestion:
The suggestion is only included for reserved prefixes.
A test has been added.