Skip to content
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.

Sign up for GitHub

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

Jump to bottom

README.md: Remove prompts from code blocks #76883

Merged
merged 1 commit into from Sep 20, 2020
Merged

README.md: Remove prompts from code blocks #76883

merged 1 commit into from Sep 20, 2020

Conversation

ghost
Copy link

@ghost ghost commented Sep 18, 2020

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (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 rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 18, 2020
@jyn514 jyn514 added the A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools label Sep 18, 2020
pickfire
pickfire reviewed Sep 18, 2020
View reviewed changes
Copy link
Contributor

@pickfire pickfire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have much opinion on this but I believe it is there for a reason and I prefer the way it is currently to be clearer ($ for unix sh and > for msvc).

@ghost
Copy link
Author

ghost commented Sep 18, 2020

($ for unix sh and > for msvc).

$ doesn't mean unix sh. I can put anything in want in prompt.

@pickfire
Copy link
Contributor

$ doesn't mean unix sh. I can put anything in want in prompt.

That's correct, fish could be > but $ usually mean a normal shell and # for a root shell. I just mean the normal unix shell you will see is $.

@ghost ghost closed this Sep 18, 2020
@pickfire
Copy link
Contributor

@qlcom Ah, you don't need to close it if you think it is good. If you think the change is better, you can ask the docs team to review, I am not from any team and just discussing this from my viewpoint.

@Mark-Simulacrum
Copy link
Member

I personally find these really annoying when trying to copy/paste because you have to carefully not select them, so I think this is a win.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 18, 2020

📌 Commit 18ce4c1 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 18, 2020
@pickfire
Copy link
Contributor

pickfire commented Sep 18, 2020 •
edited
Loading

@Mark-Simulacrum Note that one line here is not being able to copy paste without human intervention like

    pacman -Sy pacman-mirrors
    ...

If users happened to copy paste

   # Update package mirrors (may be needed if you have a fresh install of MSYS2)
   pacman -Sy pacman-mirrors

   # Install build tools needed for Rust. If you're building a 32-bit compiler,
   # then replace "x86_64" below with "i686". If you've already got git, python,
   # or CMake installed and in PATH you can remove them from this list. Note
   # that it is important that you do **not** use the 'python2', 'cmake' and 'ninja'
   # packages from the 'msys2' subsystem. The build has historically been known
   # to fail with these packages.
   pacman -S git \
               make \
               diffutils \
               tar \
               mingw-w64-x86_64-python \
               mingw-w64-x86_64-cmake \
               mingw-w64-x86_64-gcc \
               mingw-w64-x86_64-ninja

If they copy pasted the whole thing it would not work.

@Mark-Simulacrum
Copy link
Member

Right, but selecting the single-line statements is still much easier (for me) without the prefix, which IMO doesn't add much value.

@ghost ghost closed this Sep 18, 2020
@ghost
Copy link
Author

ghost commented Sep 18, 2020

@Mark-Simulacrum Note that one line here is not being able to copy paste without human intervention like

    pacman -Sy pacman-mirrors
    ...

If users happened to copy paste

   # Update package mirrors (may be needed if you have a fresh install of MSYS2)
   pacman -Sy pacman-mirrors

   # Install build tools needed for Rust. If you're building a 32-bit compiler,
   # then replace "x86_64" below with "i686". If you've already got git, python,
   # or CMake installed and in PATH you can remove them from this list. Note
   # that it is important that you do **not** use the 'python2', 'cmake' and 'ninja'
   # packages from the 'msys2' subsystem. The build has historically been known
   # to fail with these packages.
   pacman -S git \
               make \
               diffutils \
               tar \
               mingw-w64-x86_64-python \
               mingw-w64-x86_64-cmake \
               mingw-w64-x86_64-gcc \
               mingw-w64-x86_64-ninja

If they copy pasted the whole thing it would not work.

on which shell?

@Mark-Simulacrum
Copy link
Member

@qlcom Did you intentionally close this? I continue to feel that it should be merged.

@ghost ghost reopened this Sep 18, 2020
@ghost
Copy link
Author

ghost commented Sep 18, 2020

why does it need to run CI server to merge a pull request which changes a markdown file?

@Mark-Simulacrum
Copy link
Member

Everything that lands into Rust master needs to pass CI, currently we do not try to optimize for "definitely green" changes because that's error-prone and might cause failures if humans are in that loop. This'll get merged soon, though, it just needs to work its way through the queue: https://bors.rust-lang.org/homu/queue/rust

@tesuji tesuji mentioned this pull request Sep 20, 2020
Closed
@hbina
Copy link
Contributor

hbina commented Sep 20, 2020

AFAIK there's a similar discussion in rustc-dev-guide here rust-lang/rustc-dev-guide#460

RalfJung added a commit to RalfJung/rust that referenced this pull request Sep 20, 2020
@RalfJung
Rollup merge of rust-lang#76883 - qlcom:master, r=Mark-Simulacrum
ba7142b 
README.md: Remove prompts from code blocks

rust-lang#76863
This was referenced Sep 20, 2020
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 20, 2020
@bors
Auto merge of rust-lang#76975 - RalfJung:rollup-s2wiuqr, r=RalfJung
81e0270 
Rollup of 15 pull requests

Successful merges:

 - rust-lang#76732 (Add docs for `BasicBlock`)
 - rust-lang#76832 (Let backends define custom targets)
 - rust-lang#76866 (Remove unused feature gates from library/ crates)
 - rust-lang#76875 (Move to intra-doc links in library/alloc/src/collections/binary_heap.rs)
 - rust-lang#76876 (Move to intra-doc links in collections/btree/map.rs and collections/linked_list.rs)
 - rust-lang#76877 (Move to intra-doc links in collections/vec_deque.rs and collections/vec_deque/drain.rs)
 - rust-lang#76878 (Move the version number to a plaintext file)
 - rust-lang#76883 (README.md: Remove prompts from code blocks)
 - rust-lang#76887 (Add missing examples on HashSet iter types)
 - rust-lang#76890 (use matches!() macro for simple if let conditions)
 - rust-lang#76891 (don't take `TyCtxt` by reference)
 - rust-lang#76910 (transmute: use diagnostic item)
 - rust-lang#76924 (Add tracking issue for feature(unix_socket_peek))
 - rust-lang#76926 (BTreeMap: code readability tweaks)
 - rust-lang#76940 (Don't allow implementing trait directly on type-alias-impl-trait)

Failed merges:

r? `@ghost`
@bors bors merged commit 3ef093b into rust-lang:master Sep 20, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Mark-Simulacrum Mark-Simulacrum

Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.
7 participants
@rust-highfive @pickfire @Mark-Simulacrum @bors @hbina @jyn514 @rustbot