Skip to content

Add missing urls and examples for Condvar docs - #40033

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:condvar-docs
Feb 28, 2017
Merged

Add missing urls and examples for Condvar docs#40033
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:condvar-docs

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented Feb 22, 2017

Copy link
Copy Markdown
Member

frewsxcv
frewsxcv reviewed Feb 22, 2017
View reviewed changes
Comment threadsrc/libstd/sync/condvar.rs Outdated

@frewsxcvfrewsxcvFeb 22, 2017

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.

There's a lot going on in the examples in this pull request, and I understand they probably can't get much smaller, but is it possible you could add a short sentence before each example giving an overview of what's going on?

In this particular example, maybe: "Using a CondVar to notify the main thread when when a child thread has started:"

@GuillaumeGomezGuillaumeGomezFeb 22, 2017

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.

I wonder: is a summary enough or wouldn't it be better to just add more comments?

@frewsxcvfrewsxcvFeb 22, 2017

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.

Up to you, though I don't think it would hurt to have both

@GuillaumeGomezGuillaumeGomezFeb 22, 2017

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.

Let's do both then!

@GuillaumeGomez
GuillaumeGomezforce-pushed the condvar-docs branch 2 times, most recently from b30735c to b16ce2cCompareFebruary 22, 2017 17:54
@GuillaumeGomez

GuillaumeGomez commented Feb 22, 2017•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

So apparently, the WaitTimeoutResult returned from wait_timeout returns always true on timed_out function if it happens once. Not sure if this is expected or not...

PS: the code sample is here.

@GuillaumeGomez

GuillaumeGomez commented Feb 25, 2017

Copy link
Copy Markdown
MemberAuthor

cc @frewsxcv

@frewsxcv

frewsxcv commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Feb 28, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit d3b8f56 has been approved by frewsxcv

@eddybeddyb mentioned this pull request Feb 28, 2017
Closed
eddyb added a commit to eddyb/rust that referenced this pull request Feb 28, 2017
@eddyb
Rollup merge ofrust-lang#40033- GuillaumeGomez:condvar-docs, r=frew…
e72bc00
…sxcv
Add missing urls and examples for Condvar docs
r? @frewsxcv
@eddybeddyb mentioned this pull request Feb 28, 2017
Closed
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Feb 28, 2017
@frewsxcv
Rollup merge ofrust-lang#40033- GuillaumeGomez:condvar-docs, r=frew…
5be0d9f
…sxcv
Add missing urls and examples for Condvar docs
r? @frewsxcv
@frewsxcvfrewsxcv mentioned this pull request Feb 28, 2017
Merged
bors added a commit that referenced this pull request Feb 28, 2017
@bors
Auto merge of#40148- frewsxcv:rollup, r=frewsxcv
be76056
Rollup of 9 pull requests
- Successful merges: #39977, #40033, #40047, #40056, #40057, #40122, #40124, #40126, #40131
- Failed merges: #40101
@bors
bors merged commit d3b8f56 into rust-lang:masterFeb 28, 2017
@GuillaumeGomez
GuillaumeGomez deleted the condvar-docs branch February 28, 2017 20:35
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@frewsxcvfrewsxcvfrewsxcv left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@frewsxcvfrewsxcv

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GuillaumeGomez@frewsxcv@bors