Skip to content

feat: implement sync::Barrier #240

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

Merged
merged 1 commit into from
Sep 26, 2019

Conversation

dignifiedquire
Copy link
Member

@dignifiedquire dignifiedquire commented Sep 25, 2019

Based on the implementation in tokio-rs/tokio#1571

Based on the implementation in the std lib

Things to discuss

  • async or std Arc in examples and tests
  • async or std Mutex in the implementation can't use std, as it needs to be shared
  • use of broadcaster dependency to implement, well broadcast

To fix

  • missing debug implementation, once broadcaster has it, or it is dropped

Docs

Screenshot 2019-09-25 at 21 07 25

Based on the implementation in tokio-rs/tokio#1571
Copy link
Contributor

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

This patch looks good to me. I think we should accept this, and I'll file a follow-up PR to mark this as "unstable". The reason for that is that we're currently pulling in quite a few deps, and Stjepan hasn't had a chance to review the API here either (though I think we're okay).

To reduce the amount of deps we have to build I've also filed a patch against Broadcaster: leo60228/broadcaster#1.

Either way; this is fantastic work, thanks so much for putting the time in to build this!

@yoshuawuyts
Copy link
Contributor

It looks like it's Cargo that's failing CI on MacOS. All other tests' pass, so I think this should be fine. Let's see if Bors accepts this.

bors r+

bors bot added a commit that referenced this pull request Sep 26, 2019
240: feat: implement sync::Barrier r=yoshuawuyts a=dignifiedquire

~~Based on the implementation in tokio-rs/tokio#1571

Based on the implementation in the std lib 

Things to discuss

- `async` or `std` `Arc` in examples and tests
- ~~`async` or `std` `Mutex` in the implementation~~ can't use std, as it needs to be shared
- use of `broadcaster` dependency to implement, well broadcast

To fix
- [x] missing debug implementation, once broadcaster has it, or it is dropped

## Docs

![Screenshot 2019-09-25 at 21 07 25](https://user-images.githubusercontent.com/790842/65631833-a541e580-dfd8-11e9-970b-8a1df9155529.png)


Co-authored-by: dignifiedquire <dignifiedquire@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Sep 26, 2019

Build failed

  • continuous-integration/travis-ci/push

@yoshuawuyts
Copy link
Contributor

bors r+

bors bot added a commit that referenced this pull request Sep 26, 2019
240: feat: implement sync::Barrier r=yoshuawuyts a=dignifiedquire

~~Based on the implementation in tokio-rs/tokio#1571

Based on the implementation in the std lib 

Things to discuss

- `async` or `std` `Arc` in examples and tests
- ~~`async` or `std` `Mutex` in the implementation~~ can't use std, as it needs to be shared
- use of `broadcaster` dependency to implement, well broadcast

To fix
- [x] missing debug implementation, once broadcaster has it, or it is dropped

## Docs

![Screenshot 2019-09-25 at 21 07 25](https://user-images.githubusercontent.com/790842/65631833-a541e580-dfd8-11e9-970b-8a1df9155529.png)


Co-authored-by: dignifiedquire <dignifiedquire@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Sep 26, 2019

Build failed

  • continuous-integration/travis-ci/push

@yoshuawuyts
Copy link
Contributor

Looks like on this nightly cargo doc broke. Think this is okay to merge manually.

@yoshuawuyts yoshuawuyts merged commit 0f0b354 into async-rs:master Sep 26, 2019
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.

2 participants