Skip to content
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

PromiseSource and Barrier are the same thing #38796

Closed
jrieken opened this issue Nov 20, 2017 · 7 comments
Closed

PromiseSource and Barrier are the same thing #38796

jrieken opened this issue Nov 20, 2017 · 7 comments
Assignees

Comments

@jrieken
Copy link
Member

jrieken commented Nov 20, 2017

We PromiseSource and Barrier and I am very sure they both do the same. Please let only one survive.

@joaomoreno
Copy link
Member

We must FIGHT to the end.

@joaomoreno
Copy link
Member

I don't understand though... Are they really the same thing?

@jrieken
Copy link
Member Author

jrieken commented Nov 21, 2017

I believe so... Both expose a promise that can be resolved from the outside Barrier#open vs PromiseSource#complete.

@alexdima
Copy link
Member

I prefer to keep Barrier, because I like to think of it as a separate synchronization mechanism than promises, even if it uses promises as its implementation.

So, I would live with the "duplication".

@jrieken
Copy link
Member Author

jrieken commented Nov 21, 2017

Well, the single usage of the PromiseSource seems like a good usage for Barrier because it seems to synchronise on the shared process startup/spawning

@alexdima
Copy link
Member

👍 To replace the usage of PromiseSource with Barrier in SharedProcess.

@joaomoreno It does look like you have a barrier type of usage going on in the SharedProcess class. The only implementation diff I can spot is that the PromiseSource can be canceled, but that might be an implementation mistake. The PromiseSource can also be errored, but that appears to be not used.

@joaomoreno
Copy link
Member

joaomoreno commented Nov 21, 2017

Well, there is a wait() method which returns a Promise... so Promises don't seem to be just an implementation detail. Also, I don't think the word Barrier conveys the right meaning to the functionality it provides.

But sure. All for the glory of less code. I had to move it down, so I just replaced PromiseSource with it in vs/base/common/async.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants