search

pulyaevskiy / firebase-functions-interop

Firebase Functions Interop Library for Dart.
BSD 3-Clause "New" or "Revised" License
191 stars 52 forks source link

Generics, improve interop for val and set #3

Closed kevmoo closed 6 years ago

kevmoo commented 6 years ago

CC @pulyaevskiy – thoughts on testing?

pulyaevskiy commented 6 years ago

Hey @kevmoo! I'm traveling so a bit slow to respond. Will take a look when have time, most likely tomorrow.

Thanks for the PR!

pulyaevskiy commented 6 years ago

@kevmoo thanks for waiting!

I've added some basic infrastructure for integration testing but I'm not sure if it's the best way to cover this kind of logic.

We probably also need to add support for the stubs provided in JS package, described here: https://firebase.google.com/docs/functions/unit-testing This would be a bigger project though, so it might be best to do in a separate PR.

I'm still struggling with JS interop and not exactly sure how to properly bind to such things like namespaced constructors, e.g. new functions.database.DeltaSnapshot(...). Might need to change the way how bindings are done currently.

Please let me know if you have other ideas!

kevmoo commented 6 years ago

Still want to pull this in?

pulyaevskiy commented 6 years ago

Yes, I think it's a valuable change. There are a couple comments in regards to JSON decode/encode, when we resolve them I'll merge.

kevmoo commented 6 years ago

There are a couple comments in regards to JSON decode/encode, when we resolve them I'll merge.

I don't see any comments 🤷‍♂️

pulyaevskiy commented 6 years ago

Someone needed to learn how to use Github... Should see now?

pulyaevskiy commented 6 years ago

LGTM. Thanks for your contribution!