android-fhir icon indicating copy to clipboard operation
android-fhir copied to clipboard

Update gradle to 8.8 and AGP to 8.5.0

Open jingtang10 opened this issue 1 year ago • 4 comments

IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).

Description Update gradle to 8.8 and AGP to 8.5.0

Alternative(s) considered NA

Type Builds

Screenshots (if applicable)

Checklist

  • [x] I have read and acknowledged the Code of conduct.
  • [x] I have read the Contributing page.
  • [x] I have signed the Google Individual CLA, or I am covered by my company's Corporate CLA.
  • [x] I have discussed my proposed solution with code owners in the linked issue(s) and we have agreed upon the general approach.
  • [x] I have run ./gradlew spotlessApply and ./gradlew spotlessCheck to check my code follows the style guide of this project.
  • [x] I have run ./gradlew check and ./gradlew connectedCheck to test my changes locally.
  • [x] I have built and run the demo app(s) to verify my change fixes the issue and/or does not break the demo app(s).

jingtang10 avatar Jun 18 '24 12:06 jingtang10

addressed comments thanks aditya for reviewing

jingtang10 avatar Jun 18 '24 14:06 jingtang10

After taking a look at the failure:

Lint found 2 errors, 38 warnings. First failure:
[1185](https://github.com/google/android-fhir/actions/runs/9566698928/job/26372611822?pr=2575#step:5:1186)
[1186](https://github.com/google/android-fhir/actions/runs/9566698928/job/26372611822?pr=2575#step:5:1187)/home/runner/work/android-fhir/android-fhir/datacapture/src/main/java/com/google/android/fhir/datacapture/extensions/MoreLocalDates.kt:128: Error: Implicit cast from IsoChronology to Chronology requires API level 26 (current min is 24) [NewApi]
[1187](https://github.com/google/android-fhir/actions/runs/9566698928/job/26372611822?pr=2575#step:5:1188)    IsoChronology.INSTANCE,
[1188](https://github.com/google/android-fhir/actions/runs/9566698928/job/26372611822?pr=2575#step:5:1189)    ~~~~~~~~~~~~~~~~~~~~~~

and trying to fix it without updating min sdk version, I realised we do need the ISOChronology class in order to get the localized date time pattern... without using this class, it's possible to generate the date time formatter object that is localized, but we won't be able to get the actually localized pattern e.g. yyyy-mm-dd (by calling the api getLocalizedDateTimePattern)... but we do need this pattern itself as the hint text...

now an alternative is to not use that as hint text, but instead use a formatted date e.g. 2019-09-12... which might be ok but it will just diverge a bit from the behaviour we have when the entry format string is provided by the questionnaire.

so i think the easiest thing to do is actually update the minsdk.. which i think wasn't an issue only because of the build tool wasn't picking it up since the api was indeed introduced in api level 26... so this way we're only making what was already the case explicit... (device on 24 would have had problem with this already).

jingtang10 avatar Jun 19 '24 18:06 jingtang10

We dropped the minSdk (for all libraries) to 24 due to this issue #2077.

MJ1998 avatar Jun 24 '24 05:06 MJ1998

Just to confirm, we're fine raising the min level to 26

pld avatar Jun 26 '24 13:06 pld

discussed today at the developers call and no objection so far to this api level change.

if we don't hear anything else we'll include this change in the next release

jingtang10 avatar Jul 11 '24 10:07 jingtang10