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

JDeferred 2.0 - Roadmap #115

Open
17 tasks done
saturnism opened this issue Jun 21, 2017 · 9 comments
Open
17 tasks done

JDeferred 2.0 - Roadmap #115

saturnism opened this issue Jun 21, 2017 · 9 comments
Assignees
Milestone

Comments

@saturnism
Copy link
Member

saturnism commented Jun 21, 2017

Nice to have, possibly 2.1:

@saturnism
Copy link
Member Author

@aalmiray I feel some of these things were done in the last 2.x branch merge. I updated the list, PTAL.

@saturnism
Copy link
Member Author

@aalmiray the exceptions handlers needs to be registered with every promise, specifically in AbstractPromise in the triggerXYZ() methods. If we pass the reference in a constructor, that'll mean every subclass will have to get it and will change a lot of signatures. Moreover, promises that aren't created by DeferredManager won't register.

Alternatively, we can have a static global class that holds the exception handler and every promise instances would just refer to that. This would work w/ all promises even those created outside of DeferredManager. wdyt?

@aalmiray
Copy link
Contributor

The problem with a static global location is thread-safety and synchronization. We could "cheat" by using ThreadLocal but can't guarantee it will work 100% of the times. My MO when braking APIs between major versions is "if we're going to break compatibility we do it in the most spectacular way in order to ensure a better API, that is, sometimes we have to burn bridges while supplying fusion powevered hovercrafts to cross the river ;-)"

@saturnism
Copy link
Member Author

on the other hand, associating it to every deferred object might be too much for a crosscutting concern though. let's discuss more at baselone 😄

@saturnism
Copy link
Member Author

saturnism commented Oct 28, 2017

@aalmiray regarding the convenience method, the best I have so far is Promises.resolve(...) and Promises.reject(...). I can't think of anything else at the moment. Should we still include it?

They can also be part of DeferredManager, e.g., dm.resolve(...)

@aalmiray
Copy link
Contributor

I think so, as it's the only way to support resolved/rejected promises with Iterable for example. that way there's no longer a need to update the current impl to be able to handle a resolved value, as it'll be wrapped with a Promise 😄

@saturnism
Copy link
Member Author

Do you feel it should be part of DeferredManager, or, a new class w/ static methods, e.g.Promises?

@aalmiray
Copy link
Contributor

My gut feeling tells me it should be part of DeferredManager for now, as that's the place where Promises are created, other than explicit construction via DeferredObject. There are only 2 methods for now, don't think that warrants a Promises or PromiseUtil utility class just yet. If it comes to that, we can re-route DM to the utility class at a later stage.

@saturnism
Copy link
Member Author

i think we are almost done :D

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

No branches or pull requests

2 participants