apptentive / apptentive-android

Apptentive Android SDK
http://www.apptentive.com
BSD 3-Clause "New" or "Revised" License
65 stars 64 forks source link

Framework for Pluggable Ratings #10

Closed jwriteclub closed 12 years ago

jwriteclub commented 12 years ago

Changed RatingProvider and associated classes to support pluggable rating providers so that markets other than the Google Market can be used with Apptentive.

NOTE: The MiKandiRating provider is not yet complete, I'll issue another pull request once we have the code to support external ratings launch live in our market.

skykelsey commented 12 years ago

Thanks Dr. Cocktor. I'll take a look at these changes. If you have any questions or comments, please email me directly at sky@apptentive.com. And please excuse the messy code -- I have been changing things around quite a bit.

Sky

On Tue, Nov 29, 2011 at 10:20 PM, jwriteclub < reply@reply.github.com

wrote:

Changed RatingProvider and associated classes to support pluggable rating providers so that markets other than the Google Market can be used with Apptentive.

NOTE: The MiKandiRating provider is not yet complete, I'll issue another pull request once we have the code to support external ratings launch live in our market.

You can merge this Pull Request by running:

git pull https://github.com/mikandi/apptentive-android master

Or you can view, comment on it, or merge it online at:

https://github.com/apptentive/apptentive-android/pull/10

-- Commit Summary --

  • Added a TODO regarding Android 1.6
  • Fixed dead code and reference problems.
  • Updated RatingController and associated code to support pluggable rating providers allowing for markets other than the GoogleMarket to work with Apptentive.

-- File Changes --

M apptentive-android-sdk/src/com/apptentive/android/sdk/Apptentive.java (78) M apptentive-android-sdk/src/com/apptentive/android/sdk/model/GlobalInfo.java (14) A apptentive-android-sdk/src/com/apptentive/android/sdk/module/rating/IRatingProvider.java (33) A apptentive-android-sdk/src/com/apptentive/android/sdk/module/rating/InsufficientRatingArgumentsException.java (21) M apptentive-android-sdk/src/com/apptentive/android/sdk/module/rating/RatingController.java (50) A apptentive-android-sdk/src/com/apptentive/android/sdk/module/rating/impl/GoogleMarketRating.java (29) A apptentive-android-sdk/src/com/apptentive/android/sdk/module/rating/impl/MiKandiMarketRating.java (30) M apptentive-android-sdk/src/com/apptentive/android/sdk/util/EmailUtil.java (3)

-- Patch Links --

https://github.com/apptentive/apptentive-android/pull/10.patch https://github.com/apptentive/apptentive-android/pull/10.diff


Reply to this email directly or view it on GitHub: https://github.com/apptentive/apptentive-android/pull/10

jwriteclub commented 12 years ago

Sounds good. Make sure you get commit 9240aca -- I hadn't really thought the error case through all the way.

skykelsey commented 12 years ago

Thanks again. I probably won't get to integrating this until we get the Android Market/Amazon App Market plan sorted out.

Sky

On Tue, Nov 29, 2011 at 10:54 PM, jwriteclub < reply@reply.github.com

wrote:

Sounds good. Make sure you get commit 9240aca -- I hadn't really thought the error case through all the way.


Reply to this email directly or view it on GitHub:

https://github.com/apptentive/apptentive-android/pull/10#issuecomment-2955845

jwriteclub commented 12 years ago

I've merged a bunch of your changes back in.

But I've stuck with using a public interface for IRatingProvider. Why? By using private classes internal to RatingModule, a developer would have to completely re-write that piece of code in order to use a custom rating provider, which would also require forking your code for all practical purposes.

By using a public interface, on the other hand, they can simply implement IRatingProvider in their own code without needing to change the underlying apptentive library.

~ Christopher

skykelsey commented 12 years ago

Since so much changed, I'm closing this. Please open a new one with the changes we talked about.