set applicability #53655
set applicability #53655
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @estebank (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
I saw a couple of places where we need to fix the wording of the suggestion.
The changes seem correct. Can you go through all of the changes and make sure they look kind of like this?
err.span_suggestion_with_applicability(
span,
msg,
code,
applicability,
);
Other than that r=me.
src/librustc_resolve/lib.rs
Outdated
| sugg_msg, | ||
| new_snippet); | ||
| new_snippet, | ||
| Applicability::MachineApplicable); |
There was a problem hiding this comment.
Can you fix the formatting as you did below? (each arg on it's own line, 4 spaces indented, trailing comma on every line, closing ); on it's own line)
src/librustc_resolve/lib.rs
Outdated
| span, | ||
| "you can try using the variant's enum", | ||
| enum_path, | ||
| Applicability::MachineApplicable); |
There was a problem hiding this comment.
nitpick: move the closing ); to it's own line and leave a trailing comma here
|
Thanks for reviewing @estebank ! This is my first PR against |
|
@jcpst thank you for the change! This is very useful. The only thing that I would bother you with is if you could squash all your changes onto one commit (this will require you to do a force push on your repo, double check that everything went as you expected before doing so). |
|
☔ The latest upstream changes (presumably #53662) made this pull request unmergeable. Please resolve the merge conflicts. |
|
There was also a merge conflict now, so you will have to rebase against latest master. |
jcpst
force-pushed
the
with_applicability
branch
from
c6bf70a
to
4ef21be
Compare
This comment has been minimized.
This comment has been minimized.
|
Thanks for the help, although I didn't mean to add those submodule changes. Unfortunately, while researching and trying to figure out how to remove them from my PR, I seem to have my local copy in an undesirable state. Not sure how I remove them The best idea I have right now seems to be starting over with these changes. |
|
You can nuke your local branch and create a fresh one with the same name, apply your changes and force push. That will overwrite what appears in the PR. Just don't lose your changes 😊 |
jcpst
force-pushed
the
with_applicability
branch
2 times, most recently
from
137da37
to
8c6d151
Compare
This comment has been minimized.
This comment has been minimized.
jcpst
force-pushed
the
with_applicability
branch
from
8c6d151
to
1f421d6
Compare
|
Ok, I think I have the process down now. Let me know if this needs any other changes! |
|
@bors r+ rollup |
|
📌 Commit 1f421d6 has been approved by |
bors
added
S-waiting-on-bors
set applicability Update a few more calls as described in rust-lang#50723 r? @estebank
set applicability Update a few more calls as described in rust-lang#50723 r? @estebank
set applicability Update a few more calls as described in rust-lang#50723 r? @estebank
set applicability Update a few more calls as described in rust-lang#50723 r? @estebank
set applicability Update a few more calls as described in rust-lang#50723 r? @estebank
Rollup of 20 pull requests Successful merges: - #51760 (Add another PartialEq example) - #53113 (Add example for Cow) - #53129 (remove `let x = baz` which was obscuring the real error) - #53389 (document effect of join on memory ordering) - #53472 (Use FxHash{Map,Set} instead of the default Hash{Map,Set} everywhere in rustc.) - #53476 (Add partialeq implementation for TryFromIntError type) - #53513 (Force-inline `shallow_resolve` at its hottest call site.) - #53655 (set applicability) - #53702 (Fix stabilisation version for macro_vis_matcher.) - #53727 (Do not suggest dereferencing in macro) - #53732 (save-analysis: Differentiate foreign functions and statics.) - #53740 (add llvm-readobj to llvm-tools-preview) - #53743 (fix a typo: taget_env -> target_env) - #53747 (Rustdoc fixes) - #53753 (expand keep-stage --help text) - #53756 (Fix typo in comment) - #53768 (move file-extension based .gitignore down to src/) - #53785 (Fix a comment in src/libcore/slice/mod.rs) - #53786 (Replace usages of 'bad_style' with 'nonstandard_style'.) - #53806 (Fix UI issues on Implementations on Foreign types) Failed merges: r? @ghost
Update a few more calls as described in #50723
r? @estebank