Skip to content

Remove unnecessary section on the quickstart doc #384

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

Merged
merged 1 commit into from
Jun 20, 2021
Merged

Remove unnecessary section on the quickstart doc #384

merged 1 commit into from
Jun 20, 2021

Conversation

JohnTitor
Copy link

@JohnTitor JohnTitor commented Jun 20, 2021

Rustdoc is part of rustc component and we don't need an additional one.

Signed-off-by: Yuki Okushi [email protected]

@ksquirrel

This comment has been minimized.

@ksquirrel

This comment has been minimized.

@JohnTitor JohnTitor changed the title Fix rustup component name for rustdoc Remove unnecessary section on the quickstart doc Jun 20, 2021
Rustdoc is part of rustc component and we don't need an additional one.

Signed-off-by: Yuki Okushi <[email protected]>
@ksquirrel
Copy link
Member

Review of df9158a39b69:

  • ✔️ Commit df9158a: Looks fine!

@kloenk
Copy link
Member

kloenk commented Jun 20, 2021

It is in the rustup default. But for example it is not always default in the NixOS env. So I would like to keep it. As it also has the note that per default it is installed.

@JohnTitor
Copy link
Author

@kloenk As bjorn3 mentioned, all the profiles have the rustdoc even if it's minimal.

@ojeda
Copy link
Member

ojeda commented Jun 20, 2021

LGTM -- I would update https://github.com/rust-lang/rustup/blob/master/doc/src/concepts/components.md to clarify in which component rustdoc is.

@ojeda
Copy link
Member

ojeda commented Jun 20, 2021

rust-lang/rustup#2801

@ojeda
Copy link
Member

ojeda commented Jun 20, 2021

What do you mean by:

and we don't need an additional one.

in the commit message?

@JohnTitor
Copy link
Author

What do you mean by:

and we don't need an additional one.

in the commit message?

I mean we don't need an additional component (i.e. rust-docs or rustdoc). If it's unclear, I'm open to any suggestions.

@ojeda
Copy link
Member

ojeda commented Jun 20, 2021

That's fine :)

@ojeda ojeda merged commit cb9b4a3 into Rust-for-Linux:rust Jun 20, 2021
@JohnTitor JohnTitor deleted the fix-rustup-component-name branch June 20, 2021 17:16
@kloenk
Copy link
Member

kloenk commented Jun 21, 2021

@kloenk As bjorn3 mentioned, all the profiles have the rustdoc even if it's minimal.

Yes for rustup. My package manager provides a package for rustc, which only includes rustc and no rustdoc. I already got bitten by this once, as rustc and rustdoc had ben a different version.

@ojeda
Copy link
Member

ojeda commented Jun 21, 2021

Hmm... That is a good point. I think we should mention it (rustdoc is already in the table in changes.rst, but I think it should be in Quick Start too). On the other hand, we cannot try to cover every single case (e.g. someone could also want instructions to build the Rust toolchain by hand etc.).

Let me think about it.

@JohnTitor
Copy link
Author

On the other hand, we cannot try to cover every single case

I agree with this point, we treat Rustup as our recommend way to use Rust. If you want to see the specific instructions for rustdoc, then why not expand clippy and rustfmt sections as well.
That said, it might be a bit unfriendly not to mention rustdoc entirely. I've reworded the section like this: https://github.com/JohnTitor/linux/tree/mention-rustdoc
Does it work for you?

@ojeda
Copy link
Member

ojeda commented Jun 21, 2021

I agree with this point, we treat Rustup as our recommend way to use Rust.

Side-note: Rustup may not be the recommended way for the kernel later on (e.g. when distributions package the required toolchain to build their kernel).

Does it work for you?

LGTM -- perhaps remove the very last sentence since we do not say it for others:

If you are using another way to use Rust, you may have to install it manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants