/rust

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

TRPL: guessing game #25080

Merged
merged 1 commit into from May 6, 2015

Conversation

Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
None yet
Projects
None yet
Milestone
No milestone
9 participants
@steveklabnik @rust-highfive @killercup @alexcrichton @bors @solson @huonw @parir @pcwalton
@steveklabnik
Member

steveklabnik commented May 3, 2015

This also made me realize that I wasn't using the correct term,
'associated functions', rather than 'static methods'. So I corrected
that in the method syntax chapter.

@rust-highfive rust-highfive assigned pcwalton May 3, 2015

@rust-highfive

This comment has been minimized.

Show comment
Hide comment
@rust-highfive

rust-highfive May 3, 2015

Collaborator

r? @pcwalton

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

rust-highfive commented May 3, 2015

r? @pcwalton

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

@steveklabnik steveklabnik assigned alexcrichton and unassigned pcwalton May 3, 2015

Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
src/doc/trpl/guessing-game.md
let input = io::stdin().read_line(&mut guess)
.ok()
.expect("Failed to read line");

This comment has been minimized.

Show comment
Hide comment
@killercup

killercup May 3, 2015

Member

I'd probably put the read_line call on a new line as well. Or put .ok().expect(... on the same line. (Same below, where you explain stdin, then read_line, then ok().explain.)
Copy URL
@killercup

killercup May 3, 2015

Member

I'd probably put the read_line call on a new line as well. Or put .ok().expect(... on the same line. (Same below, where you explain stdin, then read_line, then ok().explain.)

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 3, 2015

Member

yeah, it's debateable. I think i'll leave it like this for now, though. This is, of course, ridiculously subjective.
Copy URL
@steveklabnik

steveklabnik May 3, 2015

Member

yeah, it's debateable. I think i'll leave it like this for now, though. This is, of course, ridiculously subjective.
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
@killercup

This comment has been minimized.

Show comment
Hide comment
@killercup

killercup May 3, 2015

Member

Good read. I've added a few comments for questions and typos. Feel free to ignore my stylistic suggestions :)
Member

killercup commented May 3, 2015

Good read. I've added a few comments for questions and typos. Feel free to ignore my stylistic suggestions :)
@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 3, 2015

Member

Thanks @killercup and @parir ! nits addressed
Member

steveklabnik commented May 3, 2015

Thanks @killercup and @parir ! nits addressed
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
Show outdated Hide outdated src/doc/trpl/guessing-game.md
@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 4, 2015

Member

Amazing @steveklabnik!

Only a few stylistic nits from me, but otherwise r=me
Member

alexcrichton commented May 4, 2015

Amazing @steveklabnik!

Only a few stylistic nits from me, but otherwise r=me
@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 4, 2015

Member

Woot! About to board a plane, but I'll get on it tomorrow. I gotta get used to linking to more docs :)
Member

steveklabnik commented May 4, 2015

Woot! About to board a plane, but I'll get on it tomorrow. I gotta get used to linking to more docs :)
src/doc/trpl/guessing-game.md
rather than do all the work of figuring out versions again. This lets you
have a repeatable build automatically.
What about when we _do_ want to use `v0.4.0`? Cargo has another command,

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 4, 2015

Member

NB. if the code above is changed to rand = "0.3" this section will need updating.
Copy URL
@huonw

huonw May 4, 2015

Member

NB. if the code above is changed to rand = "0.3" this section will need updating.

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 5, 2015

Member

Will it? rand = 0.3 will use 0.4 when it comes out, just like this.
Copy URL
@steveklabnik

steveklabnik May 5, 2015

Member

Will it? rand = 0.3 will use 0.4 when it comes out, just like this.

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 5, 2015

Member

The specification "0.3" to cargo indicates "0.3 compatible" and because 0.4 is not compatible with 0.3 it won't automatically pick it up (e.g. I believe @huonw is correct here)
Copy URL
@alexcrichton

alexcrichton May 5, 2015

Member

The specification "0.3" to cargo indicates "0.3 compatible" and because 0.4 is not compatible with 0.3 it won't automatically pick it up (e.g. I believe @huonw is correct here)

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 5, 2015

Member

"0.3.0" is shorthand for "^0.3.0" still, right? That should pick up 0.4.0, to be compatible with ^ requirements. It won't pick up a 1.0.0.
Copy URL
@steveklabnik

steveklabnik May 5, 2015

Member

"0.3.0" is shorthand for "^0.3.0" still, right? That should pick up 0.4.0, to be compatible with ^ requirements. It won't pick up a 1.0.0.

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 5, 2015

Member

Yes, but 0.3.0 is not semver compatible with 0.4.0, so it won't pick it up
Copy URL
@alexcrichton

alexcrichton May 5, 2015

Member

Yes, but 0.3.0 is not semver compatible with 0.4.0, so it won't pick it up

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 5, 2015

Member

Where's this behavior documented? Breaking ^ seems really bad :/
Copy URL
@steveklabnik

steveklabnik May 5, 2015

Member

Where's this behavior documented? Breaking ^ seems really bad :/

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 5, 2015

Member

Cargo takes 0.x to be the major releases for 0. versions, otherwise there wouldn't be a way to make incompatible releases other than going straight to 1.0. (docs)

In fact, strictly speaking even this is different to what semver really is:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

I.e. 0.3.1 is "meant" to be not compatible with 0.3.2, although I prefer our scheme.
Copy URL
@huonw

huonw May 5, 2015

Member

Cargo takes 0.x to be the major releases for 0. versions, otherwise there wouldn't be a way to make incompatible releases other than going straight to 1.0. (docs)

In fact, strictly speaking even this is different to what semver really is:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

I.e. 0.3.1 is "meant" to be not compatible with 0.3.2, although I prefer our scheme.

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 6, 2015

Member

Gotcha. Ouch :/ well, i'll write a patch for this later today, since this PR was merged. Thanks you two!
Copy URL
@steveklabnik

steveklabnik May 6, 2015

Member

Gotcha. Ouch :/ well, i'll write a patch for this later today, since this PR was merged. Thanks you two!

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 6, 2015

Member

#25141
Copy URL
@steveklabnik

steveklabnik May 6, 2015

Member

#25141
src/doc/trpl/guessing-game.md
takes a reference to the thing you want to compare it to. It returns the
`Ordering` type we `use`d earlier. We use a [`match`][match] statement to
determine exactly what kind of `Ordering` it is. `Ordering` is an
[`enum`][enum], short for ‘enumeration’, which looks like this:

This comment has been minimized.

Show comment
Hide comment
@huonw

huonw May 4, 2015

Member

Hm, this sentence is a bit confusing, since Ordering doesn't look like the enum below.
Copy URL
@huonw

huonw May 4, 2015

Member

Hm, this sentence is a bit confusing, since Ordering doesn't look like the enum below.
Show outdated Hide outdated src/doc/trpl/guessing-game.md
@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 5, 2015

Member

@bors: r=alexcrichton rollup
Member

steveklabnik commented May 5, 2015

@bors: r=alexcrichton rollup
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 5, 2015

Contributor

📌 Commit ef1a123 has been approved by alexcrichton
Contributor

bors commented May 5, 2015

📌 Commit ef1a123 has been approved by alexcrichton

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 5, 2015

@steveklabnik
Rollup merge of #25080 - steveklabnik:guessing_game, r=alexcrichton 
This also made me realize that I wasn't using the correct term,
'associated functions', rather than 'static methods'. So I corrected
that in the method syntax chapter.
56955e5

@steveklabnik steveklabnik referenced this pull request May 5, 2015

Closed

Rollup of 4 pull requests #25122

bors added a commit that referenced this pull request May 5, 2015

@bors
Auto merge of #25122 - steveklabnik:rollup, r=steveklabnik 
- Successful merges: #24782, #25080, #25112, #25114
- Failed merges:
ac837bb
@steveklabnik
TRPL: guessing game 
This also made me realize that I wasn't using the correct term,
'associated functions', rather than 'static methods'. So I corrected
that in the method syntax chapter.
9d512fd
@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@steveklabnik

steveklabnik May 5, 2015

Member

@bors: r=alexcrichton rollup
Member

steveklabnik commented May 5, 2015

@bors: r=alexcrichton rollup
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 5, 2015

Contributor

📌 Commit 9d512fd has been approved by alexcrichton
Contributor

bors commented May 5, 2015

📌 Commit 9d512fd has been approved by alexcrichton

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 5, 2015

@steveklabnik
Rollup merge of #25080 - steveklabnik:guessing_game, r=alexcrichton 
This also made me realize that I wasn't using the correct term,
'associated functions', rather than 'static methods'. So I corrected
that in the method syntax chapter.
477cf9c

@steveklabnik steveklabnik referenced this pull request May 5, 2015

Merged

Rollup of 5 pull requests #25129

bors added a commit that referenced this pull request May 5, 2015

@bors
Auto merge of #25129 - steveklabnik:rollup, r=steveklabnik 
- Successful merges: #24782, #25080, #25112, #25114, #25127
- Failed merges:
252b544

@bors bors merged commit 9d512fd into rust-lang:master May 6, 2015
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment