Remove unneeded methods from api #109
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 69.30% 70.54% +1.23%
==========================================
Files 21 21
Lines 404 404
Branches 49 49
==========================================
+ Hits 280 285 +5
+ Misses 110 104 -6
- Partials 14 15 +1 ☔ View full report in Codecov by Sentry. |
| @@ -14,11 +14,6 @@ public final class com/parsely/parselyandroid/ParselyNotInitializedException : j | |||
|
|
|||
There was a problem hiding this comment.
This file comes from #101 and highlights how the public API changed.
wzieba
requested review from
oguzkocer and
iangmaia
and removed request for
oguzkocer
| ParselyTracker.sharedInstance().resetVideo(); | ||
| } | ||
|
|
||
| private boolean engagementIsActive() { |
There was a problem hiding this comment.
Are these private API calls really useful in the demo app? Meaning: does it make sense to show this info given it's not going to be available for clients?
I get that the intention might be just showing some debug info, but if so, perhaps good to at least add a comment highlighting that these are private methods and not intended to be used by users in general? 🤔
There was a problem hiding this comment.
Are these private API calls really useful in the demo app?
I think yes - it might be helpful to see how the internal parameters of the SDK behave after certain actions to see how it works. Especially, as things happen here after intervals 10+ seconds.
But I see that it's the "example" app, so it should be giving a good example of the integration. I agree that in the current form it could be misleading.
In 003d6fb I've extracted all calls to a separate class and added comments. WDYT?
There was a problem hiding this comment.
But I see that it's the "example" app, so it should be giving a good example of the integration
🎯 Exactly!
In 003d6fb I've extracted all calls to a separate class and added comments. WDYT?
This is a good compromise IMO, and it makes it clear to clients that this is just for debugging 👍 Thanks for taking the time to address it!
Description
This PR removes some methods from the public API. Those methods are not needed to use the SDK and were making the API less readable. Internal communication: p1706537769574079-slack-C0533SEJ82H
Reflection invocation
Those methods, although not needed for public API, are useful in the scope of the demo app. That's why I've decided to keep them in
ParselyTrackerInternaland invoke them using reflection.Testing
Install the
exampleapp and tap on each button. The app should not crash.