search

matomo-org / matomo-sdk-android

SDK for Android to measure your apps with Matomo. Works on Android phones, tablets, Fire TV sticks, and more!
BSD 3-Clause "New" or "Revised" License
390 stars 162 forks source link

Remove lint warnings #337

Closed hannesa2 closed 2 years ago

hannesa2 commented 2 years ago

I followed the suggestion from lint

codecov-commenter commented 2 years ago

Codecov Report

Merging #337 (ecab9c7) into master (37d78d7) will increase coverage by 0.30%. The diff coverage is 90.00%.

@@             Coverage Diff              @@
##             master     #337      +/-   ##
============================================
+ Coverage     86.78%   87.09%   +0.30%     
  Complexity      371      371              
============================================
  Files            33       32       -1     
  Lines          1415     1410       -5     
  Branches        165      164       -1     
============================================
  Hits           1228     1228              
+ Misses          117      113       -4     
+ Partials         70       69       -1
Impacted Files Coverage Δ
tracker/src/main/java/org/matomo/sdk/Matomo.java 86.84% <ø> (ø)
...r/src/main/java/org/matomo/sdk/TrackerBuilder.java 79.31% <ø> (ø)
...java/org/matomo/sdk/dispatcher/EventDiskCache.java 73.07% <0.00%> (ø)
...ain/java/org/matomo/sdk/extra/CustomVariables.java 94.87% <ø> (ø)
...a/org/matomo/sdk/extra/MatomoExceptionHandler.java 83.33% <ø> (ø)
...main/java/org/matomo/sdk/LegacySettingsPorter.java 100.00% <100.00%> (ø)
tracker/src/main/java/org/matomo/sdk/TrackMe.java 100.00% <100.00%> (ø)
...org/matomo/sdk/dispatcher/DefaultPacketSender.java 68.57% <100.00%> (+0.51%) :arrow_up:
...ain/java/org/matomo/sdk/extra/DownloadTracker.java 75.00% <100.00%> (+2.02%) :arrow_up:
...r/src/main/java/org/matomo/sdk/tools/Checksum.java 95.45% <100.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 37d78d7...ecab9c7. Read the comment docs.

d4rken commented 2 years ago

Till now, I ignored my wish to kotlize it. Kotlin has some wonderful mechanism for a sdk, e.g internal

You could do a Kotlin rewrite if you want :). Would probably looks very nice. Requires a major version bump and some taking care that the APIs do not totally break for people using this with Java still.

hannesa2 commented 2 years ago

Nice, we come closer

image
hannesa2 commented 2 years ago

Would probably looks very nice. Requires a major version bump and some taking care that the APIs do not totally break for people using this with Java still.

In an ideal world, the api is done by interfaces and anything behind will be done internal. As long as you don't change the interface you can convert without any doubt what you want.

This keeping unused classes because someone could use it, is an afford without benefit.

hannesa2 commented 2 years ago

But this interface way isn't done

image
d4rken commented 2 years ago

This keeping unused classes because someone could use it, is an afford without benefit.

Why do you keep coming back to this? :smile: I know it's not great, I probably just forgot about that class. I had no one to review my code. It's out there now and we shouldn't break the users app "just because it's not clean". Sorry for not writing the perfect SDK? :thinking:

But this interface way isn't done

Not everything needs to be a firebase-level SDK with shovels of abstraction. This is volunteer work and the SDK worked fine for my use-cases, so at that time: Why make things more complicated if there is no requirement? A rewrite to Kotlin wasn't on the horizon at that point.

Interfaces can still be introduced now if desired. As said, you can make a v5, with everything new and better if you want to :+1:.

hannesa2 commented 2 years ago

Sorry, those were just a few thoughts on the Kotlin transformation. You are right, house holding of our time is also important.