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

adjust java code example for updating your Application (#4387) #4397

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

DerBasler
Copy link

No description provided.

Copy link

netlify bot commented Dec 9, 2024

Deploy Preview for react-native ready!

Name Link
🔨 Latest commit 84ebed7
🔍 Latest deploy log https://app.netlify.com/sites/react-native/deploys/675ff0f7ada79500082afc26
😎 Deploy Preview https://deploy-preview-4397--react-native.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment on lines 316 to 320
+ try {
+ SoLoader.init(this, OpenSourceMergedSoMapping.INSTANCE);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this?

Copy link
Author

Choose a reason for hiding this comment

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

Because facebook/react-native#47633 (comment) in my experience it did not work without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah but the IOException catch is not necessary

Copy link
Author

Choose a reason for hiding this comment

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

I agree since I catch and just rethrow it, so it is useless but without it the build fails:
error: unreported exception IOException; must be caught or declared to be thrown
SoLoader.init(this, OpenSourceMergedSoMapping.INSTANCE);

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. That's a bug inside SoLoader and we should fix it rather than having to tweak the documentation

Copy link
Contributor

Choose a reason for hiding this comment

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

cortinico added a commit to cortinico/SoLoader that referenced this pull request Dec 10, 2024
Summary:
This mimics the same behavior of `init(Context, boolean)` and lifts the requirement for
Java consumers to catch an exception explicitely.

See facebook/react-native-website#4397

Differential Revision: D67032849
cortinico added a commit to cortinico/SoLoader that referenced this pull request Dec 10, 2024
…k#135)

Summary:

This mimics the same behavior of `init(Context, boolean)` and lifts the requirement for
Java consumers to catch an exception explicitely.

See facebook/react-native-website#4397

Reviewed By: cipolleschi

Differential Revision: D67032849
facebook-github-bot pushed a commit to facebook/SoLoader that referenced this pull request Dec 10, 2024
Summary:
Pull Request resolved: #135

This mimics the same behavior of `init(Context, boolean)` and lifts the requirement for
Java consumers to catch an exception explicitely.

See facebook/react-native-website#4397

Reviewed By: cipolleschi, Abbondanzo

Differential Revision: D67032849

fbshipit-source-id: 2128e6762f33c52df6f60e2c57a0963bbda5beea
@cortinico
Copy link
Contributor

We would need for the SoLoader release though

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.

3 participants