Bumped targetSdkVersion and compileSdkVersion #3357
Conversation
|
There is still a question I asked on slack about |
| @@ -218,8 +218,7 @@ public void setTitle(CharSequence title) { | |||
| */ | |||
| @SuppressWarnings("deprecation") | |||
| private void fixSpinner(Context context, int hourOfDay, int minute, boolean is24HourView) { | |||
| // android:timePickerMode spinner and clock began in Lollipop | |||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { | |||
| if (Build.VERSION.SDK_INT == Build.VERSION_CODES.N) { | |||
There was a problem hiding this comment.
How did you determine this is now only relevant for N?
I think the block in https://github.com/opendatakit/collect/blob/4eb3b2b1d7be3e62ae249e450c526a0673d5b40b/collect_app/src/main/java/org/odk/collect/android/widgets/TimeWidget.java#L253 should also be updated accordingly since that test doesn't make sense anymore.
There was a problem hiding this comment.
How did you determine this is now only relevant for N?
The gist you have used is also updated in the same way: https://gist.github.com/jeffdgr8/6bc5f990bf0c13a7334ce385d482af9f
and also in https://issuetracker.google.com/issues/37119315 you can find that it's fin in API 25
I think the block in should also be updated accordingly since that test doesn't make sense anymore.
right.
|
I see @opendatakit/testers are in favor of limiting the API level range! It's true that it would be really nice. It's the kind of decision that is major enough that the TSC should consider it and that the community should be told a bit ahead. That means it's definitely not going to be for v1.24. |
grzesiek2010
force-pushed
the
COLLECT-3189
branch
from
4eb3b2b to
4a8a9b9
Compare
|
Ahhh I missed one page, maybe even the most important one since is related to our app most:
It's not something super simple so I would vote for bumping to 28 (the upcoming release) and then to 29. |
grzesiek2010
force-pushed
the
COLLECT-3189
branch
from
a9f5688 to
28e6d48
Compare
|
@mmarciniak90 I did a rebase so that it includes the fix for #3362 |
|
I couldn't find a bug in this PR but I noticed 3 other bugs during testing this issue.
I also confirm that this doesn't mean that fix is without problems so it will be better to merge it and have the opportunity to work with these changes all the time. Verified on Android: 4.2, 4.4, 5.1, 6.0, 7.0, 8.1, 9.0 Verified cases:
@opendatakit-bot unlabel "needs testing" |
Closes #3189
What has been done to verify that this works as intended?
I investigated the doc:
https://developer.android.com/about/versions/pie/android-9.0-changes-28
https://developer.android.com/about/versions/pie/android-9.0-changes-all
https://developer.android.com/about/versions/10/behavior-changes-10
https://developer.android.com/about/versions/10/behavior-changes-all
also, I tested the app a bit:
Why is this the best possible solution? Were any other approaches considered?
We just have to update targetSdkVersion to 28 so we decided to update to 29 to test everything just once.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
I wasn't able to find anything apart from the issue with TimeWidget but it doesn't mean that it's the only problem. Bugs might be everywhere...
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.