Skip to content

Conversation

@Chaoba
Copy link
Contributor

@Chaoba Chaoba commented Aug 16, 2021

Included changes:

  • LeacyCanary, update to 2.7, no need to config for release and test. No need any config in codes.
  • Multidex, methods num exceed limit after bump all the dependency.
  • Add android:exported="true" in manifest and config for PendingIntent for Android 12
  • Bump maps sdk to 9.6.2 which contains the telemetry fix for Android 12

@Chaoba Chaoba self-assigned this Aug 16, 2021
@Chaoba Chaoba requested a review from a team August 16, 2021 11:23
Copy link
Contributor

@kiryldz kiryldz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Chaoba could you update PR description why actually leak canary was dropped, several dependencies were added, multidex was enabled. Is it all connected with updating to Android 12?

@Chaoba
Copy link
Contributor Author

Chaoba commented Aug 17, 2021

A separate ticket is created to track the failed release job.

@kiryldz
Copy link
Contributor

kiryldz commented Aug 17, 2021

@Chaoba is it possible to separate this PR in 2 PRs:

  1. Android 12 compatibility
  2. All other work done here.

I see no sense in messing it all up in one PR despite changes for leak canary, firebase etc are indeed related to Android 12 compatibility.
If CI does not pass in general - guess we need to firstly merge PR with fixing it and then apply separate PR for fixing Android 12.

@Chaoba Chaoba requested a review from kiryldz August 17, 2021 11:39
Comment on lines +55 to +57
implementation (dependenciesList.mapboxGeocoding, {
exclude group: 'com.squareup.okhttp3'
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we exclude the okhttp specifically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will conflict with another version of okhttp and cause a compile error.

@Chaoba Chaoba merged commit 17379d2 into main Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants