-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Removed @no-rustfix annotation and updated suggestions to multipart_suggestion
as required by #13099
#13216
Conversation
…uggestion as required by #13098
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @llogiq (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 (
|
The fmt test is now successful, but some UI tests fail. Running |
@llogiq I have run |
You will have to adapt the lint implementation(s) to use a different style of creating those lint suggestions, so that they can be applied to the test files automatically. I would be really surprised if just removing the annotations will magically work, because then we wouldn't have added them in the first place. |
The libz error is odd, does it work after a |
Even in the master branch, |
@Alexendoo I am running this code on the ubuntu machine. |
What's the output of |
@Alexendoo Here is the screenshot. |
Looks like it is a known dependency - rust-lang/rust#74420, I believe you'd want to install You could mention it in that issue that you ran into this error |
Add a test for ice-3717.rs this PR is a part of #13099. Based on the changes introduced in #13098 for introduce ui_test, we will update the uitest output. This is a fix for `ice-3717.rs`. Although fixes have already been made in #13216, it seems that he is a first-time contributor. I thought it might be better for him to refer to my PR, so I created it accordingly. Since this is my first contribution in a while, please let me know if there are any issues or required changes. changelog: None r! `@flip1995`
Add a test for ice-3717.rs this PR is a part of #13099. Based on the changes introduced in #13098 for introduce ui_test, we will update the uitest output. This is a fix for `ice-3717.rs`. Although fixes have already been made in #13216, it seems that he is a first-time contributor. I thought it might be better for him to refer to my PR, so I created it accordingly. Since this is my first contribution in a while, please let me know if there are any issues or required changes. changelog: none r! `@flip1995`
@ibilalkayy In PR #13098, all tests that were expected to split into multipart files have been deleted and set to be skipped using You remove https://github.com/rust-lang/rust-clippy/blob/master/book/src/development/writing_tests.md In my PR, multiple files were not generated, but I believe that in this issue, it is expected that multiple files will be generated by the multipart suggestion. |
Triage: Seems like this PR went stale. As it doesn't really fix the issue, I'm closing it. Thanks for the explanation @kyoto7250 , I copied your comment to the tracking issue, as I think that it explains the task at hand way better than what I wrote in the issue description! |
Hey there, I removed the
@no-rustfix annotation and updated suggestions to multipart_suggestion
line from the files that were mentioned in the #13099 issue. I removed the@no-rustfix
annotation and mentioned it in the PR also but I didn't understand the 3rd point because I didn't saw any suggestion point except these comments.Let me know if I am missing anything.
Although there was a test error in the fmt but I didn't modified the
tests/check-fmt.rs
file. Here is the screenshot of it..stderr
file)cargo test
passes locallycargo dev update_lints
cargo dev fmt
Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.
Delete this line and everything above before opening your PR.
Please write a short comment explaining your change (or "none" for internal only changes)
changelog: