search

parse-community / Parse-SDK-Android

The Android SDK for Parse Platform
https://parseplatform.org/
Other
1.88k stars 739 forks source link

fix: Missing Proguard rules for R8 in full mode #1196

Closed azlekov closed 10 months ago

azlekov commented 1 year ago

New Pull Request Checklist

Issue Description

Closes: #1194

Approach

TODOs before merging

parse-github-assistant[bot] commented 1 year ago

I will reformat the title to use the proper commit message syntax.

parse-github-assistant[bot] commented 1 year ago

Thanks for opening this pull request!

azlekov commented 1 year ago

Looks good

Is this ready for merge? I've changed this to a feat PR, since it seems to add support for Gradle 8 to the Parse SDK.

That's not quite correct. But fixes some release issues with ProGuard and R8

mtrezza commented 1 year ago

I assumed so because the issues don't seem to have these occurred so far. Is this an adaptation for Gradle 8 or is this also a bug for Gradle <8?

mtrezza commented 1 year ago

Maybe we should start adding a gradle matrix to the CI where we test with different gradle versions - would that make sense?

azlekov commented 1 year ago

Hey @mtrezza the issues which the developers are reporting are runtime. Because of wrong ProGuard rules after magnification is done Parse classes are erased and when you run the app it crashes. I need to think about if there can be some tests covering the cases by building and running simple app using parse via some instrumentation test and check whatever it crash or not.

mtrezza commented 1 year ago

Ok, so the minification is the core issue here. If we can't find a test (soon) we can also just merge this fix for now and open a new issue to add a test, so we don't forget it.

Other than that, wouod it still male sense to run the CI with multiple Gradle or Android versions? If so, I'll open a separate issue for that.

mtrezza commented 10 months ago

@azlekov This PR got somewhat stale, can we merge this or is there anything that is worth discussing beforehand?

codecov[bot] commented 10 months ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (f9843c5) 0.00% compared to head (40a3fdf) 0.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1196 +/- ## ====================================== Coverage 0.00% 0.00% ====================================== Files 122 122 Lines 9971 9971 Branches 1345 1345 ====================================== Misses 9971 9971 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

parse-github-assistant[bot] commented 10 months ago

I will reformat the title to use the proper commit message syntax.

azlekov commented 10 months ago

@mtrezza I change the title to be more precise, it's not a feat, but a fix for proguard R8 in full mode as mentioned in the #1194

mtrezza commented 10 months ago

Great! I've just slightly edited the title as it should describe the actual issue. Otherwise I think this is ready for merge?

azlekov commented 10 months ago

Great! I've just slightly edited the title as it should describe the actual issue. Otherwise I think this is ready for merge?

Yes, go go go! 🚗

parseplatformorg commented 10 months ago

🎉 This change has been released in version 4.2.1