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

feat(ionicnative): add instance wrapper #97

Merged
merged 17 commits into from
Apr 30, 2016
Merged

feat(ionicnative): add instance wrapper #97

merged 17 commits into from
Apr 30, 2016

Conversation

ihadeed
Copy link
Collaborator

@ihadeed ihadeed commented Mar 29, 2016

closes #86
closes #79

@tlancina Came up with this over night, it worked with the SQLite plugin. Haven't tested with other plugins. Let me know what you think.

Example of implementation: https://github.com/driftyco/ionic-native/blob/instance-wrapper/src/plugins/sqlite.ts

@tlancina
Copy link
Contributor

Nice! I'd maybe remove the console logs and have CordovaInstance call wrapInstance directly so you can get rid of options.instanceMethod, but other than that looks good. We have nothing for this right now, so might as well get it in and test it out and see what the issues are.

@keithdmoore
Copy link
Contributor

keithdmoore commented Apr 18, 2016

@ihadeed Where are you at on this one? I am looking to add keychain and will need to use something like this. Should I use the instance_wrapper branch or are you pretty close to pushing these changes to master?

@ihadeed
Copy link
Collaborator Author

ihadeed commented Apr 18, 2016

Hey Keith,

If I remember correctly, the concept I had in mind is already implemented
but it hasn't been tested thoroughly.

I'm using the instance_wrapper branch to develop the wrapper and some of
the plugins that would rely on it.

I would suggest to develop on that branch so we can alter the instance
wrapper in case of any problems.
On Apr 18, 2016 11:44 AM, "Keith D. Moore" notifications@github.com wrote:

@ihadeed https://github.com/ihadeed Where are you at on this one? I am
looking to add keychain and will need to use something like this. Should I
use the instance_wrapper branch or are you pretty close to push these
changes to master?


You are receiving this because you were mentioned.

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

@ihadeed ihadeed merged commit 8b6d9f3 into master Apr 30, 2016
@ihadeed ihadeed deleted the instance-wrapper branch April 30, 2016 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discuss: A good way to implement instance based plugins Cordova SQLite Plugin
3 participants