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

Editing Chapter 1 #29538

Merged
merged 3 commits into from
Nov 5, 2015
Merged

Editing Chapter 1 #29538

merged 3 commits into from
Nov 5, 2015

Conversation

steveklabnik
Copy link
Member

I did some preliminary editing work with No Starch on the first chapter of the book, and here's some of the results. We're going to want to return to this later when @brson etc's new rustup work is done, so this is mostly just a first pass.

But, we agreed that having separate chapters for each of this bit of intro is a bit excessive. So let's move all of this intro stuff into one chapter.

I'd appreciate a careful review of this, as there was also some confusion about some things, which resulted in me taking one huge markdown file apart and splitting it back up, as well as some editor issues, so I think this looks good, but double checking things matters!

/cc @aturon

Lots of little details and things.
@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

Also note that the function body is wrapped in curly braces (`{` and `}`). Rust
requires these around all function bodies. It's considered good style to put
the opening curly brace on the same line as the function declaration, with one
line space in between.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this just be "space" instead of "line space"?

Conceptually, this makes more sense as one introductory chapter.
@steveklabnik
Copy link
Member Author

Fixed up @apasel422 's comments, as well as fixing the build 😓

| `x86_64-unknown-dragonfly` | ✓ | ✓ | | 64-bit DragonFlyBSD |
| `x86_64-rumprun-netbsd` | ✓ | | | 64-bit NetBSD Rump Kernel |
| `i686-pc-windows-msvc` (XP) | ✓ | | | Windows XP support |
| `x86_64-pc-windows-msvc` (XP) | ✓ | | | Windows XP support |
Copy link
Contributor

Choose a reason for hiding this comment

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

This list is sure to be insta-outdated in dead-tree format...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I am not entirely sure what to do about that, exactly.

@brson
Copy link
Contributor

brson commented Nov 5, 2015

r=me

@steveklabnik
Copy link
Member Author

@bors: r=brson rollup

@bors
Copy link
Contributor

bors commented Nov 5, 2015

📌 Commit 7bb193c has been approved by brson

@steveklabnik
Copy link
Member Author

Thanks @brson! I didn't leave a comment for every change, but I should have addressed it all.

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Nov 5, 2015
I did some preliminary editing work with No Starch on the first chapter of the book, and here's some of the results. We're going to want to return to this later when @brson etc's new rustup work is done, so this is mostly just a first pass.

But, we agreed that having separate chapters for each of this bit of intro is a bit excessive. So let's move all of this intro stuff into one chapter.

I'd appreciate a careful review of this, as there was also some confusion about some things, which resulted in me taking one huge markdown file apart and splitting it back up, as well as some editor issues, so I _think_ this looks good, but double checking things matters!

/cc @aturon
bors added a commit that referenced this pull request Nov 5, 2015
@bors bors merged commit 7bb193c into rust-lang:master Nov 5, 2015
ollie27 added a commit to ollie27/rust that referenced this pull request Jan 25, 2016
@steveklabnik steveklabnik deleted the ch1_edits branch June 19, 2016 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants