Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Proguard rules #81

Merged
merged 8 commits into from
Mar 13, 2018
Merged

Update Proguard rules #81

merged 8 commits into from
Mar 13, 2018

Conversation

electrostat
Copy link
Contributor

Add in rules to handle warnings when utilizing proguard (unable to find certain play service classes) in builds

- add gms rules to location to handle issues
- ignore warning during compile
- keep files for run time calls
@langsmith
Copy link
Contributor

langsmith commented Mar 9, 2018

cc'ing to follow @langsmith

- move proguard rules to core instead of location
- add in MockLocationEngine, color, and Lost rules
@electrostat electrostat changed the title Add Play Service Proguard rules Update Proguard rules Mar 12, 2018
- remove mocklocationengine
- remove MAS dependency
- update snapshot branch
- change snapshot name
@electrostat electrostat requested review from zugaldia, Guardiola31337, tobrun and LukasPaczos and removed request for Guardiola31337 March 12, 2018 17:24
- revert test-snapshot changes
- remove lost dependency and classes
Copy link
Member

@tobrun tobrun left a comment

Choose a reason for hiding this comment

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

Looks great, could we add

proguardFiles getDefaultProguardFile('proguard-android.txt'), 'proguard-rules.pro'

To the debug section of the build.gradle? this is handy for testing downstream so proguard picks up the configuration when running in debug mode

-keep public class com.google.android.gms.* { public *; }
-dontwarn com.google.android.gms.**
-dontwarn java.awt.Color
-dontwarn com.mapzen.android.lost.api**
Copy link
Member

Choose a reason for hiding this comment

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

since we are removing lost from the dependencies list, we can drop this line

#-renamesourcefileattribute SourceFile
-keep public class com.google.android.gms.* { public *; }
-dontwarn com.google.android.gms.**
-dontwarn java.awt.Color
Copy link
Member

Choose a reason for hiding this comment

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

since we are removing mapboxServices, we can drop this line

- add proguard rules to debug  section
- change some rules
Copy link
Contributor

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

🚀

# If you keep the line number information, uncomment this to
# hide the original source file name.
#-renamesourcefileattribute SourceFile
-keep public class com.google.android.gms.* { public *; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if these lines are necessary 🤔
Did you see any warnings in downstream @tobrun @LukasPaczos?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Guardiola31337 If you take a look at mapbox/mapbox-gl-native#11417, there are a bunch of warnings from com.google.android.gms package.

electrostat added a commit that referenced this pull request Mar 22, 2018
* Add GMS Rules

- add gms rules to location to handle issues
- ignore warning during compile
- keep files for run time calls

* Remove MAS dependency + MockLocationEngine

- remove mocklocationengine
- remove MAS dependency

* Remove LOST

- remove lost dependency and classes

* Proguard rules in Debug and change rules

- add proguard rules to debug  section
- change some rules
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants