-
-
Notifications
You must be signed in to change notification settings - Fork 473
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
Merge plugin #47
Comments
The For my own purposes, I will probably rewrite other native sections of the plugin to fix bugs and add functionality. Before creating this fork, I asked the maintainers of Hence I decide to create this fork for my own use. You are also welcome to use it (or not). If you wish this functionality to be merged into |
Hi Dave, you are doing great and you have come so far in terms of keeping this repository up to date with latest build & changes. Well, our intention in the start was to just fix the build issue because of breaking change release by Google Firebase but on the next immediate day, we decided to maintain the repository for existing & future users by improving the code regularly. And since 17th Jun 2019, we have done the following:
With due respect, well, this definitely not touched the native code majorly but that doesn't mean we are just resolving build issues which currently exist in the original plugin. It's just a matter of time & availability. We both (at least us) have started maintaining the fork to give back to the open-source community so that the developers across the globe can benefit from it and can keep their focus on their core business logic. And our intention is still the same. Request you to please respect the work we tried to deliver so far. Your words were more like "they did nothing to maintain". Now, coming to the main point, if developers are getting confused or diverted to two different repositories, we will be happy to achieve our repository and send them to this repository. But I have a concern. You recently changed the names of some JS API which will not work with the Ionic native wrapper so the Ionic developers might not be able to use it. So are you going to update the same in the Ionic wrapper? And even if you update those Ionic TS wrappers, Ionic 3 users will not be able to use that. Please let us know your thoughts! |
@sagrawal31 I apologise if my comment offended you - that was not my intention. Since I made the leap and decided to make this fork (rather than feeding PRs back to So with regard to Ionic Native, you are correct that this plugin will no longer work with the So I emphasise again, this fork is primarily for my own purposes (to be used in my own production apps) however I'm happy to take on board feedback for other users of it since it's in my interests that other devs use my code as it increases the likelihood of bugs/issues being spotted and fixed. Ideally I would not have to create/maintain this fork if the maintainers of If I were creating a Cordova plugin wrapper for the Firebase SDK from scratch, I would have taken the componentised approach of @chemerisuk and created a separate plugin for each logical component of the Firebase SDK rather than this monolithic approach which includes components you may not need in your app build. Hope that explains my rationale and once again I apologise if you were offended as that was not my intention. |
I really appreciate your apology and in-depth reply, Dave. And I apologies too for taking your comment wrong and for this delay in response. 😄
Agreed. We can do that in the future but developers are more tend to use the native wrappers (because they feel safe using it).
Totally agreed. The original fork (and hence we are) is adding many firebase features which are of no use to everybody. So looks like we are now on the same page. So here are my questions & thoughts:
@dpa99c @hugoblanc your thoughts, please. |
@sagrawal31 Shashank, I'm glad we've sorted out that misunderstanding and I'd be happy to collaborate with you on this. I've added you to this repo as a contributor and added a credits section to the README in anticipation of your contributions 😁 - looking at the commit history in cordova-plugin-firebase-lib, there's some useful stuff in your repository that would be good to bring across to this one.
That's a good idea - I'll create an issue to capture this: maybe we should add a sub-section under Migrating from cordova-plugin-firebase in the README which explicitly states the breaking API changes. It should probably also make it clear that |
|
Nice 😄
Thank you so much for this. Really appreciate. I'll start pushing some updates to the repo and docs. |
On my side, I will work on the ionic native wrapper |
|
Thanks for everyone working hard to standardize and make a single place for Firebase. I think it will really help the community. Are there any particular steps to follow when migrating from wizpanda/cordova-plugin-firebase-lib to this plugin? I am using 5.1.1 of that plugin. Thanks! |
cordova-plugin-firebase-lib was quite equivalent to the original cordova-plugin-firebase so you could follow Migration guide. As said dpa99c: if you're ionic user: native wrapper will not work with this new plugin, but a pull request is coming |
Feature request
Title
Merge plugin
Description
Ionic users are all using the good old cordova-plugin-firebase Ionic Native firebase
Which is not a good solution as described here: Issue 3038
Your repository and the wizpanda's repository are trying to achieve the same thing.
solution
Merge both repository would be ideal.
Then we should be able to create an ionic native plugin which does not tearing up the community.
reasons
This new repo could get more visibility with the ionic documentation, and bring clarity between all cordova-plugin-firebase forks
Describe alternatives you've considered
Not merge, and create an ionic native "plugin" with only one repository. This solutions leads to dupplication of solutions and the end of the unchoosen plugin, which is sad as you both seems invested into this plugin.
Additional context
I also posted on wizpanda repo feature 45
The text was updated successfully, but these errors were encountered: