Skip to content

Add missing urls and examples for Condvar docs#40033

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

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

Conversation

@GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Feb 22, 2017

frewsxcv
frewsxcv reviewed Feb 22, 2017
View reviewed changes
src/libstd/sync/condvar.rs Outdated
Copy link
Contributor

@frewsxcv frewsxcv Feb 22, 2017

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:"

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Feb 22, 2017

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?

Copy link
Contributor

@frewsxcv frewsxcv Feb 22, 2017

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

Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Feb 22, 2017

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 GuillaumeGomez force-pushed the condvar-docs branch 2 times, most recently from b30735c to b16ce2c Compare February 22, 2017 17:54
@GuillaumeGomez
Copy link
Member Author

GuillaumeGomez commented Feb 22, 2017 •
edited
Loading

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
Copy link
Member Author

GuillaumeGomez commented Feb 25, 2017

cc @frewsxcv

@frewsxcv
Copy link
Contributor

frewsxcv commented Feb 28, 2017

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Feb 28, 2017

📌 Commit d3b8f56 has been approved by frewsxcv

@eddyb eddyb 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 of rust-lang#40033 - GuillaumeGomez:condvar-docs, r=frew…
e72bc00 
…sxcv

Add missing urls and examples for Condvar docs

r? @frewsxcv
@eddyb eddyb 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 of rust-lang#40033 - GuillaumeGomez:condvar-docs, r=frew…
5be0d9f 
…sxcv

Add missing urls and examples for Condvar docs

r? @frewsxcv
@frewsxcv frewsxcv 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:master Feb 28, 2017
@GuillaumeGomez GuillaumeGomez deleted the condvar-docs branch February 28, 2017 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@frewsxcv frewsxcv frewsxcv left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@frewsxcv frewsxcv

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@GuillaumeGomez @frewsxcv @bors