Skip to content
This repository has been archived by the owner on Sep 15, 2021. It is now read-only.

Migrate from smoke to reflectable #75

Open
dam0vm3nt opened this issue Oct 5, 2015 · 7 comments
Open

Migrate from smoke to reflectable #75

dam0vm3nt opened this issue Oct 5, 2015 · 7 comments

Comments

@dam0vm3nt
Copy link

I've done some work here. It is using @jsProxyReflectable from polymer-1.0 because this gives many advantages on using observe with polymer-1.0.

Also I've introduced a 'breaking change' of removing Symbol in favour of String.

@eernstg
Copy link

eernstg commented Oct 5, 2015

Cool -- do create some issues on package reflectable if you hit a bug!

@dam0vm3nt
Copy link
Author

No bugs at the moment. Would you like me to send you a PR (for observe package I mean)?

Il giorno lun 5 ott 2015 alle ore 17:20 Erik Ernst [email protected]
ha scritto:

Cool -- do create some issues on package reflectable if you hit a bug!


Reply to this email directly or view it on GitHub
#75 (comment).

@jmesserly
Copy link
Contributor

@dam0vm3nt Awesome! Your changes look good to me so far

@eernstg
Copy link

eernstg commented Oct 5, 2015

If you do have a good idea about how to fix something then a PR is of course nice, but often the big step is to be able to reproduce the problem. If we get some code (e.g., some git clone commands) such that we can do that then we're usually quite happy, and then there is no need for you to study a lot of unknown details in reflectable in order to find a fix for the bug.

Ah, there was an edit: 'for observe'. If you find problems in reflectable that can be reproduced then getting access to your code is certainly helpful.

@dam0vm3nt
Copy link
Author

@jmesserly I've disabled almost all the tests and created a new one to test some basic things but if you're happy with that I'll PR my changes. They works well with another "effort" I've made to let polymer-1.0 work like the old one with respect of how observe. I mean : no need to use notifyPath or set or get or add(...) or anything because they get called for you when you change the model.

@dam0vm3nt
Copy link
Author

@eernstg I was missing you until your last sentence. Yep if I hit a bug on reflectable I will certainly do it. But this issue is for observe package that it would be nice if it gets ported to reflectable instead of using the old smoke. Thanks for your kind attention anyway.

@dam0vm3nt
Copy link
Author

@jmesserly submitted a PR if you're nice with having all test disabled :-/

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

3 participants