sarriaroman / FabricPlugin

Fabric.io plugin for Cordova or Phonegap
MIT License
198 stars 159 forks source link

Include libraries and update native projects on plugin install #4

Closed Justin-Credible closed 8 years ago

Justin-Credible commented 8 years ago

We are using the cordova-crashlytics-plugin and are looking to upgrade to a newer plugin that uses the updated Fabric SDK.

There are two things this old plugin does that makes it really convenient to use.

  1. It includes the Fabric SDK libraries with the plugin.
  2. Uses an after_plugin_install build hook to modify the iOS and Android projects.

This makes it possible to use the plugin without checking in the platforms directory or doing any manual configuration steps. I would assume this would also take care of #2 as well.

Justin-Credible commented 8 years ago

I've started working on this for my own project. Before I get too far, I'm just curious if you'd accept this as a pull request to your project, instead of me creating yet another plugin on npm.

My changes will be:

sarriaroman commented 8 years ago

Please feel free to send me your PR and please add yourself and contributor.

Justin-Credible commented 8 years ago

I'm getting to close being done; I've got the automatic setup of the Fabric libraries for both iOS and Android:

https://github.com/Justin-Credible/FabricPlugin/commits/feature/auto-configure-fabric

For my own project, I ended up needing some extra logging events for Answers that did not exist yet, so I started adding them. This lead me to the .es6 files. Would you mind if I removed these and just use a plain JavaScript interface instead? While I am a fan of ES6, TypeScript, etc, it seems overkill in this case since the JavaScript methods simply delegate to cordova.exec(...). Otherwise contributors will need to setup babel to make changes to the JavaScript files. Edit: got the files to transpile with Babel 5.x.

sarriaroman commented 8 years ago

I was checking your repository, awesome job. Please create the Pull Request when you feel that the code is ready and add yourself as contributor!

Justin-Credible commented 8 years ago

I ended up going forward and implementing all of the APIs for Answers, as well as added run-time tests and TypeScript definitions.

This turned out to be a rather large refactor touching all files.

I also bumped the version to 1.0.0 since all of the Crashlytics and Answers APIs are now present.

Let me know what you think.

tomsun commented 8 years ago

@Justin-Credible @sarriaroman curious; under which license is lib/ios/* distributed and from which url at fabric's servers do they originate?

Justin-Credible commented 8 years ago

These iOS framework bundles were obtained from Cocoapods:

https://cocoapods.org/pods/Fabric https://cocoapods.org/pods/Crashlytics

Podspec references:

https://github.com/CocoaPods/Specs/blob/master/Specs/Fabric/1.6.5/Fabric.podspec.json https://github.com/CocoaPods/Specs/blob/master/Specs/Crashlytics/3.7.0/Crashlytics.podspec.json

Framework package ZIPs:

https://kit-downloads.fabric.io/cocoapods/fabric/1.6.5/fabric.zip https://kit-downloads.fabric.io/cocoapods/crashlytics/3.7.0/crashlytics.zip

I've put in a pull request to put the above information into a readme file in the lib/ios directory.

tomsun commented 8 years ago

@Justin-Credible thanks!

tomsun commented 8 years ago

@Justin-Credible You linked to crashlytics 3.7.0, while f8ba9cf3c31b827e85d1f3dcb22f054679cddf94 (which seems to be the latest update to lib/ios in master at the time of writing) refers to crashlytics 3.6.0.

Seems accurate otherwise: For peace of mind I verified with diff -r that the contents of lib/ios indeed matches what the following urls deliver to me ATM (unzipped) :+1: https://kit-downloads.fabric.io/cocoapods/fabric/1.6.5/fabric.zip https://kit-downloads.fabric.io/cocoapods/crashlytics/3.6.0/crashlytics.zip

(Maybe it would be possible to get the files on demand from Fabric's servers using some Cordova hook, more along the lines of the Cocoapods workflow and the Maven workflow for the Android conterparts? https://github.com/sarriaroman/FabricPlugin/blob/master/hooks/lib/android-helper.js#L16 - That would eliminate the question whether or not the Fabric team is OK with redistribution and would keep this repo smaller (except one version is already in the Git history). Not sure exactly how to do it though...)

Justin-Credible commented 8 years ago

@tomsun Ah! Thanks for catching that. I must have copy/pasted the wrong URL. I'll update my pull request.