search

brarcher / loyalty-card-locker

Stores your barcode-based store/loyalty cards on your phone
GNU General Public License v3.0
171 stars 29 forks source link

Up targetSdk for Google Play #334

Closed TheLastProject closed 4 years ago

TheLastProject commented 4 years ago

If I understand https://developer.android.com/distribute/best-practices/develop/target-sdk correctly, this is needed to publish an update to the Play store.

I ran the test on the new targetSdk and clicked around and could not find any obvious new issues. But I will run a targetSdk 28 build on my device for some time and report back if I experience any issues anyway. If you don't hear from me, targetSdk 28 is stable :)

This change is Reviewable

TheLastProject commented 4 years ago

Edit: Fixed in d9f97380d9cf2980a83b30f95c3ab159835ecb93.

Nevermind, some graphical glitches appeared.

Screenshot_1577464077 Screenshot_1577464089

TheLastProject commented 4 years ago

Haven't noticed any new issues pop up yet, no.

Hmm, is there any info on how to do that migration to AndroidX? It sounds like a big step?

brarcher commented 4 years ago

There is some documentation on migrating to AndroidX:

https://developer.android.com/jetpack/androidx/migrate

Android Studio has an option to refactor a project to AndroidX. I gave it a shot on top of this branch. It mostly worked. A few places it did not. Namely,

If you want, could you give this patch a try and see if you find any issues? I did not find anything when trying it with an emulator. I did not have a chance to try out the barcode capture with a camera nor the card sharing functionality.

0001-Update-to-AndroidX.patch.txt

TheLastProject commented 4 years ago

I applied the patch you attached and ran it onto my Android phone. I found no issues at all, not even with capturing a barcode with camera or sharing a loyalty card. So I've also added your patch to this pull request.

TheLastProject commented 4 years ago

Oh, every single unit test started failing. I guess we probably need to upgrade Robolectric too. I'll look into that.

TheLastProject commented 4 years ago

Well, this ended up being a few more changes than intended, but the tests work again :)

brarcher commented 4 years ago

Oh, every single unit test started failing. I guess we probably need to upgrade Robolectric too. I'll look into that.

This is going a bit down the rabbit hole. How about the following. Let's limit this pull request to updating the SDK 28. That change is needed if this app is to be updated on the play store. Updating the app to AndroidX and SDK 29 can be done in another pull request.

brarcher commented 4 years ago

Oh, I guess I did not read the last comment. Seems you did dig into the issues and fixed the tests. I'll look at those changes.

TheLastProject commented 4 years ago

Well, we may as well go to SDK 29 if we can. I upped the number and ran the unit tests (which all work) and installed the new version on my phone with no issues as well. In my testing, everything works just fine still.