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.

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

smol grammar changes to README.md #101684

Merged
merged 3 commits into from
Sep 14, 2022
Merged

smol grammar changes to README.md #101684

merged 3 commits into from
Sep 14, 2022

Conversation

zahash
Copy link
Contributor

@zahash zahash commented Sep 11, 2022

smol grammar changes to README.md

@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.

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 11, 2022
Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

Your MSYS2 change seems like a clear improvement :) I'm not sure that the other changes are actually easier to read though.

README.md Outdated
2. Run `mingw32_shell.bat` or `mingw64_shell.bat` from wherever you installed
MSYS2 (i.e. `C:\msys64`), depending on whether you want 32-bit or 64-bit
2. Run `mingw32_shell.bat` or `mingw64_shell.bat` from the MSYS2 installation
directory (i.e. `C:\msys64`), depending on whether you want 32-bit or 64-bit
Copy link
Member

Choose a reason for hiding this comment

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

while you're fixing typos, this existing one is my pet peeve

Suggested change
directory (i.e. `C:\msys64`), depending on whether you want 32-bit or 64-bit
directory (e.g. `C:\msys64`), depending on whether you want 32-bit or 64-bit

@@ -168,7 +167,7 @@ shell with:
python x.py build
```

Currently, building Rust only works with some known versions of Visual Studio. If
Right now, building Rust only works with some known versions of Visual Studio. If
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its not necessary, but i find "Right now" simpler than "Currently"

Copy link
Member

Choose a reason for hiding this comment

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

I think "currently" sounds better, "right now" sounds too much like something very temporary that might change at any second too me, but this has been a problem for quite some time I think

Copy link
Contributor Author

@zahash zahash Sep 11, 2022

Choose a reason for hiding this comment

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

i think its the opposite. "i'm not currently dating anyone" but you might in the future. "my dog isn't currently house-trained" but it might soon be.

right now just means at this present moment. it doesn't assume anything about the duration.

what do you think?

i will change it back if you want me to.

@@ -225,7 +224,7 @@ the ABI used. I.e., if the ABI was `x86_64-pc-windows-msvc`, the directory will

Since the Rust compiler is written in Rust, it must be built by a
precompiled "snapshot" version of itself (made in an earlier stage of
development). As such, source builds require a connection to the Internet, to
development). As such, source builds require an Internet connection to
Copy link
Member

Choose a reason for hiding this comment

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

why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its just simpler to say "Internet connection" than "connection to the Internet"

README.md Outdated
Comment on lines 118 to 120
1. Grab the latest [MSYS2 installer][msys2] and go through the installer.
1. Download the latest [MSYS2 installer][msys2] and go through the installer.

2. Run `mingw32_shell.bat` or `mingw64_shell.bat` from wherever you installed
MSYS2 (i.e. `C:\msys64`), depending on whether you want 32-bit or 64-bit
2. Run `mingw32_shell.bat` or `mingw64_shell.bat` from the MSYS2 installation
directory (i.e. `C:\msys64`), depending on whether you want 32-bit or 64-bit
Copy link
Member

Choose a reason for hiding this comment

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

this seems like a clear improvement, thanks :)

@jyn514 jyn514 added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 11, 2022
@jyn514 jyn514 assigned jyn514 and unassigned Mark-Simulacrum Sep 11, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 11, 2022

Ok, none of these changes seem incorrect and I buy that they're easier to understand for non-native speakers. Thanks!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 11, 2022

📌 Commit a91e618 has been approved by jyn514

It is now in the queue for this repository.

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 11, 2022
README.md Outdated
@@ -32,7 +32,7 @@ The `x.py` command can be run directly on most systems in the following format:

This is how the documentation and examples assume you are running `x.py`.

Systems such as Ubuntu 20.04 LTS do not create the necessary `python` command by default when Python is installed that allows `x.py` to be run directly. In that case you can either create a symlink for `python` (Ubuntu provides the `python-is-python3` package for this), or run `x.py` using Python itself:
Systems such as Ubuntu 20.04 LTS do not create the necessary `python` command by default when Python is installed that allows `x.py` to be run directly. In that case, you can either create a symlink for `python` (Ubuntu provides the `python-is-python3` package for this) or run `x.py` using Python itself:
Copy link
Contributor

@danielhenrymantilla danielhenrymantilla Sep 11, 2022

Choose a reason for hiding this comment

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

Suggested change
Systems such as Ubuntu 20.04 LTS do not create the necessary `python` command by default when Python is installed that allows `x.py` to be run directly. In that case, you can either create a symlink for `python` (Ubuntu provides the `python-is-python3` package for this) or run `x.py` using Python itself:
Systems such as Ubuntu 20.04 LTS do not create the necessary `python` command by default when Python is installed that allows `x.py` to be run directly. In that case, you can either create a symlink for `python` (Ubuntu provides the `python-is-python3` package for this), or run `x.py` using Python itself:
  • by virtue of being an Oxford comma, it is valid (but not necessary);
  • having a comma after a parenthesis gives the reader a much needed pause.

So I'd rather keep it🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jyn514
Copy link
Member

jyn514 commented Sep 11, 2022

@bors r-

#101684 (comment) seems reasonable, can you please fix it?

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 11, 2022
@zahash
Copy link
Contributor Author

zahash commented Sep 13, 2022

i think i made all the suggested changes. but i see that the pr is still waiting on author. is there anything i forgot to do?

@jyn514
Copy link
Member

jyn514 commented Sep 14, 2022

@zahash nope :) when you're done making changes you can use

@rustbot ready

to say it's ready for review again.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 14, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 14, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Sep 14, 2022

📌 Commit e82922f has been approved by jyn514

It is now in the queue for this repository.

@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 14, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 14, 2022
Rollup of 6 pull requests

Successful merges:

 - rust-lang#101433 (Emit a note that static bounds from HRTBs are a bug)
 - rust-lang#101684 (smol grammar changes to README.md)
 - rust-lang#101769 (rustdoc: remove redundant CSS `.out-of-band > span.since { position }`)
 - rust-lang#101772 (Also replace the placeholder for the stable_features lint)
 - rust-lang#101773 (rustdoc: remove outdated CSS `.content table` etc)
 - rust-lang#101779 (Update test output for drop tracking)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 706e4d3 into rust-lang:master Sep 14, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

8 participants