Skip to content

Keeps the org.json package.#1891

Merged
JayShortway merged 2 commits into
mainfrom
keep-org-json
Oct 29, 2024
Merged

Keeps the org.json package.#1891
JayShortway merged 2 commits into
mainfrom
keep-org-json

Conversation

@JayShortway

Copy link
Copy Markdown
Member

As the title says. This rule will normally have no effect as we don't declare the org.json:json dependency, but some apps do (transitively). In that scenario, this rule avoids NoSuchMethodErrors and NoSuchFieldErrors.

Note: I gave this the pr:fix label, as it will fix the issue reported in this Zendesk ticket. Let me know if you think we should give it another label instead.

@JayShortway JayShortway added the pr:fix A bug fix label Oct 28, 2024
@JayShortway JayShortway requested a review from a team October 28, 2024 15:36
@JayShortway JayShortway self-assigned this Oct 28, 2024
@codecov

codecov Bot commented Oct 28, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.25%. Comparing base (dad3113) to head (3673777).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1891   +/-   ##
=======================================
  Coverage   82.25%   82.25%           
=======================================
  Files         227      227           
  Lines        7968     7968           
  Branches     1121     1121           
=======================================
  Hits         6554     6554           
  Misses        967      967           
  Partials      447      447           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

# The org.json package is part of the Android framework, so the classes are always available.
# However, sometimes it gets added to the app's classpath, either explicitly or transitively. When
# this happens, it becomes susceptible to be shrunk. The following rule avoids that.
-keep class org.json.* { *; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

However, sometimes it gets added to the app's classpath, either explicitly or transitively.

Hmm is this something we're doing? As in, I'm a bit reticent to add keep rules to our SDK unless we know it's our fault...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah no it isn't, but some apps might do it. See the linked Zendesk ticket in the PR description. I updated the comment in the ProGuard file to clarify this: 3673777.

If the dependency is not included by the app, this rule does nothing. (org.json is provided by the Android OS in that case.) If the dependency is included by the app, this rule makes sure that all classes that we expect are kept. Without this rule, the dependency can be shrunk by R8/ProGuard, causing errors.

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok I think it should be ok 👍

@JayShortway JayShortway merged commit c650d3d into main Oct 29, 2024
@JayShortway JayShortway deleted the keep-org-json branch October 29, 2024 10:07
tonidero pushed a commit that referenced this pull request Nov 4, 2024
**This is an automatic release.**

## RevenueCat SDK
### ✨ New Features
* [Experimental] Web purchase redemption (#1889) via Toni Rico
(@tonidero)
### 🐞 Bugfixes
* Keeps the org.json package. (#1891) via JayShortway (@JayShortway)
### 📦 Dependency Updates
* Bump rexml from 3.3.8 to 3.3.9 (#1892) via dependabot[bot]
(@dependabot[bot])
* Bump danger from 9.5.0 to 9.5.1 (#1885) via dependabot[bot]
(@dependabot[bot])
* Bump fastlane from 2.224.0 to 2.225.0 (#1884) via dependabot[bot]
(@dependabot[bot])

## RevenueCatUI SDK
### 🐞 Bugfixes
* Fix application of modifiers in `Markdown` component (#1893) via Toni
Rico (@tonidero)

### 🔄 Other Changes
* [CI] Allow publishing snapshot releases manually from branches (#1888)
via Toni Rico (@tonidero)
* Detekt auto-fixes (#1886) via Toni Rico (@tonidero)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants