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

Fix sentence structure #73649

Merged
merged 7 commits into from
Jul 1, 2020
Merged

Fix sentence structure #73649

merged 7 commits into from
Jul 1, 2020

Conversation

Anirban166
Copy link
Contributor

Fixed grammar and sentence structure on appropriate instances.

@Anirban166
Fix sentence structure
886f81e 
Fixed grammar and sentence structure on appropriate instances.
@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 @nikomatsakis (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 Jun 23, 2020
nikomatsakis
nikomatsakis requested changes Jun 23, 2020
View reviewed changes
README.md Outdated

This is the main source code repository for [Rust]. It contains the compiler,
standard library, and documentation.
standard library and documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

heh, I prefer the Good Ol' Oxford comma, but I'm happy either way.

Copy link
Member

Choose a reason for hiding this comment

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

we generally do use the oxford comma

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, won't change the oxford ways then :)

README.md Outdated
@@ -17,8 +17,8 @@ Read ["Installation"] from [The Book].
_Note: If you wish to contribute to the compiler, you should read [this
chapter][rustcguidebuild] of the rustc-dev-guide instead of this section._

The Rust build system has a Python script called `x.py` to bootstrap building
the compiler. More information about it may be found by running `./x.py --help`
The Rust build system has a Python script called `x.py` to bootstrap when building
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this word "when", it sounds sort of strange to me...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but "bootstrap building the compiler" doesn't sound quite right either, so what would you suggest in place of that?

Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't bother me, but I never thought that carefully about it. I'm not sure we even need the word bootstrap in there, we could just say "The rust build system uses a Python script called x.py to build the compiler." If we want to, we could add "x.py manages the bootstrapping process."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, will change it to that!

README.md Outdated Show resolved Hide resolved
steveklabnik
steveklabnik reviewed Jun 23, 2020
View reviewed changes
README.md Outdated
@@ -1,7 +1,7 @@
# The Rust Programming Language
# The Rust Programming Language
Copy link
Member

Choose a reason for hiding this comment

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

this change seems extraneous.

Copy link
Contributor Author

@Anirban166 Anirban166 Jun 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry this was a misclick on my part.
But now that I think of it, a Rust logo wouldn't be bad as an addition to that. Would you like me to create a logo for it?
or like use some badge such as this for instance?

Copy link
Contributor

Choose a reason for hiding this comment

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

badge seems nice

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:D Will add that!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've replaced the h1 heading "The Rust Programming Language" with the badge which says "Rust" beside the Rust logo, with a black background wording of "Programming Language", which you can adjust it to any wording you want by replacing the word before the first dash and after it like badge/word1-word2-colour? in the link https://img.shields.io/badge/Rust-Programming%20Language-black?style=flat&logo=rust and thereafter select the color you want. (hexcodes work too I think)

I have included an image link instead of the badge notation so you can tweak with parameters such as height width etc upto your preference.

Copy link
Contributor

Choose a reason for hiding this comment

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

image

nikomatsakis
nikomatsakis approved these changes Jun 25, 2020
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

LGTM, @steveklabnik wdyt?

README.md Outdated
@@ -1,7 +1,7 @@
# The Rust Programming Language
<img height = "50" src = "https://img.shields.io/badge/Rust-Programming%20Language-black?style=flat&logo=rust">
Copy link
Contributor

Choose a reason for hiding this comment

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

we should have an alt tag, I imagine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I'll add that now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just previewed the changes in my mobile and it looked kinda cramped up so I set up responsiveness based on device width (height will be adjusted automatically)
I currently have it set to 90% in width of the viewing device, which you can change as per your convenience

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@steveklabnik Do the changes seem okay or should I revert/change them?

README.md Outdated Show resolved Hide resolved
Anirban166 and others added 4 commits June 25, 2020 16:29
@Anirban166 @nikomatsakis
Commit suggestion
22fc18f 
Co-authored-by: Niko Matsakis <[email protected]>
@Anirban166
Add alternate text for rust logo image
8edcc6d
@Anirban166
Added clickable-link
3a1ac28 
Linked the logo/svg to https://www.rust-lang.org/
(change if required)
@Anirban166
Add responsiveness to logo
4c33b7c 
Added responsiveness to image logo based on device width (set at 90% of device width, which can be changed as per requirement)
steveklabnik
steveklabnik approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@steveklabnik steveklabnik left a comment

Choose a reason for hiding this comment

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

Thanks!

@steveklabnik
Copy link
Member

@bors: r+ rollup

@bors
Copy link
Contributor

bors commented Jun 30, 2020

📌 Commit 4c33b7c has been approved by steveklabnik

@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 Jun 30, 2020
Manishearth added a commit to Manishearth/rust that referenced this pull request Jun 30, 2020
@Manishearth
Rollup merge of rust-lang#73649 - Anirban166:patch-1, r=steveklabnik
3de07e0 
Fix sentence structure

Fixed grammar and sentence structure on appropriate instances.
@Manishearth Manishearth mentioned this pull request Jun 30, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 1, 2020
@Manishearth
Rollup merge of rust-lang#73649 - Anirban166:patch-1, r=steveklabnik
5d38ad8 
Fix sentence structure

Fixed grammar and sentence structure on appropriate instances.
@Manishearth Manishearth mentioned this pull request Jul 1, 2020
Closed
Manishearth added a commit to Manishearth/rust that referenced this pull request Jul 1, 2020
@Manishearth
Rollup merge of rust-lang#73649 - Anirban166:patch-1, r=steveklabnik
9b61081 
Fix sentence structure

Fixed grammar and sentence structure on appropriate instances.
@Manishearth Manishearth mentioned this pull request Jul 1, 2020
Closed
@Manishearth Manishearth mentioned this pull request Jul 1, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 1, 2020
@bors
Auto merge of rust-lang#73924 - Manishearth:rollup-8r51ld9, r=Manishe…
f781bab 
…arth

Rollup of 17 pull requests

Successful merges:

 - rust-lang#72071 (Added detailed error code explanation for issue E0687 in Rust compiler.)
 - rust-lang#72369 (Bring net/parser.rs up to modern up to date with modern rust patterns)
 - rust-lang#72445 (Stabilize `#[track_caller]`.)
 - rust-lang#73466 (impl From<char> for String)
 - rust-lang#73548 (remove rustdoc warnings)
 - rust-lang#73649 (Fix sentence structure)
 - rust-lang#73678 (Update Box::from_raw example to generalize better)
 - rust-lang#73705 (stop taking references in Relate)
 - rust-lang#73716 (Document the static keyword)
 - rust-lang#73752 (Remap Windows ERROR_INVALID_PARAMETER to ErrorKind::InvalidInput from Other)
 - rust-lang#73776 (Move terminator to new module)
 - rust-lang#73778 (Make `likely` and `unlikely` const, gated by feature `const_unlikely`)
 - rust-lang#73805 (Document the type keyword)
 - rust-lang#73806 (Use an 'approximate' universal upper bound when reporting region errors)
 - rust-lang#73828 (Fix wording for anonymous parameter name help)
 - rust-lang#73846 (Fix comma in debug_assert! docs)
 - rust-lang#73847 (Edit cursor.prev() method docs in lexer)

Failed merges:

r? @ghost
@bors bors merged commit 0eb9799 into rust-lang:master Jul 1, 2020
@Anirban166 Anirban166 deleted the patch-1 branch July 2, 2020 09:41
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Dec 22, 2020
Merged
@cuviper cuviper added this to the 1.46 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

@steveklabnik steveklabnik steveklabnik approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
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.46
Development

Successfully merging this pull request may close these issues.
6 participants
@Anirban166 @rust-highfive @steveklabnik @bors @nikomatsakis @cuviper