Skip to content

Add missing urls in collections module - #37304

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:collections_url
Oct 25, 2016
Merged

Add missing urls in collections module#37304
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:collections_url

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented Oct 20, 2016

Copy link
Copy Markdown
Member

@steveklabnik

steveklabnik commented Oct 24, 2016

Copy link
Copy Markdown
Contributor

@bors: delegate=frewsxcv

@bors

bors commented Oct 24, 2016

Copy link
Copy Markdown
Collaborator

✌️ @frewsxcv can now approve this pull request

frewsxcv
frewsxcv approved these changes Oct 24, 2016
View reviewed changes
Comment threadsrc/libstd/collections/mod.rs
//! just inserted.
//!
//! If an `Occupied(entry)` is yielded, then the key *was* found. In this case,
//! the user has several options: they can `get`, `insert`, or `remove` the

@frewsxcvfrewsxcvOct 24, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm generally in favor of the serial comma, but I'm not sure if Rust has an official policy around this. Regardless, in this particular instance, I don't think there's any ambiguity in either scenario so I won't waste your time changing it.

@steveklabniksteveklabnikOct 24, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, generally very pro, unsure if we've made it actual policy.

@GuillaumeGomezGuillaumeGomezOct 24, 2016•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

As said on IRC: I'm in favor for more fluidity. Commas break readings and shouldn't be more present than necessary.

@frewsxcv

frewsxcv commented Oct 24, 2016

Copy link
Copy Markdown
Contributor

Looks great, thanks! ✨

@bors r+ rollup

@bors

bors commented Oct 24, 2016

Copy link
Copy Markdown
Collaborator

📌 Commit 9ec7c65 has been approved by frewsxcv

sophiajt pushed a commit to sophiajt/rust that referenced this pull request Oct 24, 2016
Jonathan Turner
Rollup merge ofrust-lang#37304- GuillaumeGomez:collections_url, r=f…
855f3e7
…rewsxcv
Add missing urls in collections module
r? @steveklabnik
@sophiajtsophiajt mentioned this pull request Oct 24, 2016
Merged
bors added a commit that referenced this pull request Oct 24, 2016
@bors
Auto merge of#37382- jonathandturner:rollup, r=jonathandturner
7a20864
Rollup of 7 pull requests
- Successful merges: #37228, #37304, #37324, #37328, #37336, #37349, #37372
- Failed merges:
@bors
bors merged commit 9ec7c65 into rust-lang:masterOct 25, 2016
@GuillaumeGomez
GuillaumeGomez deleted the collections_url branch October 25, 2016 09:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
@steveklabniksteveklabniksteveklabnik left review comments
@frewsxcvfrewsxcvfrewsxcv approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@steveklabniksteveklabnik

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GuillaumeGomez@steveklabnik@bors@frewsxcv