alesgenova / post-me

📩 Use web Workers and other Windows through a simple Promise API
https://www.npmjs.com/package/post-me
MIT License
492 stars 13 forks source link

Re arch #27

Closed franleplant closed 3 years ago

franleplant commented 3 years ago

This includes some of the changes we previously discussed.

This is still wip.

A couple of important things I've noticed

cc @alesgenova

alesgenova commented 3 years ago

I gave it a superficial look an I'm a little overwhelmed :) Do you think it would be too late to split PR into a set of smaller gradual PRs that can be reviewed and tested more easily?

For example, one for the changes to the Messenger, one for the interface names, one fore the Parent/Child class, etc

franleplant commented 3 years ago

it's kind of hard to split it because so many things are related, the only thing I can split is the messenger refactor but if you look closely I only refactored very minimally (turned function attributes into methods, turned constructor arguments into instance methods and renamed and moved acceptableEvent function.

I would suggest you download the code and take a close look at it, it is the same things I did on the old ibridge re-arch plus the things we discussed, nothing too fancy here.

I would assume that this is a starting point and perhaps an integration branch into which boths of us can start incorporating the related changes we've discussed.

Fran

On Thu, Dec 31, 2020 at 3:34 PM Alessandro Genova notifications@github.com wrote:

I gave it a superficial look an I'm a little overwhelmed :) Do you think it would be too late to split PR into a set of smaller gradual PRs that can be reviewed and tested more easily?

For example, one for the changes to the Messenger, one for the interface names, one fore the Parent/Child class, etc

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ibridge-js/ibridge/pull/27#issuecomment-753023325, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOEIJ2E4632AENCECALJ33SXS74DANCNFSM4VPUALFQ .