-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Make the docs clearer for new contributors #5874
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthiaskrgr (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. |
This is already described in |
r? @flip1995 |
doc/basics.md
Outdated
**Note:** You may get compiler errors when building Clippy, even if you | ||
didn't change anything. That's because, as mentioned earlier, Clippy | ||
depends on rustc internals. There is [discussion][nightly-build-issue] | ||
on ways to make sure that rustc does not break Clippy in the future | ||
so that Clippy always builds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we mention that this is called a tool breakage and link to forge docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is different from classic tool breakages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I guess I didn't realize that. Are there docs that talk about the differences between classic tool breakages and clippy tool breakages?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, see https://rustc-dev-guide.rust-lang.org/contributing.html#external-dependencies-subtree and the following section.
TL;DR: Clippy breakages will never appear in nightlies delivered with rustups, so they aren't user facing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So when there's an internal breaking change, clippy is updated in rust-lang/rust, but then has to be brought over to this repository (rust-lang/rust-clippy)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly.
* Add an easy-to-see note at the top of `CONTRIBUTING.md` that points new contributors to the Basics docs * Add a note about compiler errors as a result of internals changes that break Clippy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out and improving the doc! No we're waiting for the rustup.
@bors r+ rollup |
📌 Commit bd71b01 has been approved by |
Rollup of 5 pull requests Successful merges: - #5825 (Add the new lint `same_item_push`) - #5869 (New lint against `Self` as an arbitrary self type) - #5870 (enable #[allow(clippy::unsafe_derive_deserialize)]) - #5871 (Lint .min(x).max(y) with x < y) - #5874 (Make the docs clearer for new contributors) Failed merges: r? @ghost changelog: rollup
It confused me before, so I made it extra obvious that you need to run
a script to set up your toolchain before you can build Clippy.
I also added a note so that new contributors aren't confused when
Clippy doesn't build as a result of a change in rustc's internals.
changelog: make
CONTRIBUTING.md
clearer for new contributors