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

bump fresco to 2.3.0 #30061

Closed
wants to merge 4 commits into from
Closed

Conversation

dulmandakh
Copy link
Contributor

Summary

Bump Fresco to 2.3.0.

Changelog

[Android] [Changed] - Bump Fresco to 2.3.0

Test Plan

@react-native-bot react-native-bot added the Platform: Android Android applications. label Sep 29, 2020
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. labels Sep 29, 2020
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 06ce643

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@fkgozali has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@fkgozali
Copy link
Contributor

fkgozali commented Oct 2, 2020

Hmm actually, test_android failure seems related:

java.lang.NoClassDefFoundError: Could not initialize class com.facebook.imagepipeline.platform.KitKatPurgeableDecoder

Can you double check?

@dulmandakh
Copy link
Contributor Author

It looks like failing to load native libraries, and I'm not good at Buck to resolve the issue. Could you please ask someone in FB for help.

@fkgozali
Copy link
Contributor

fkgozali commented Oct 2, 2020

I'll find time to investigate next week, but expect some delay.

@mikehardy
Copy link
Contributor

I maintain a lot of repos and I dislike it when people just "bump" things, at the same time, I am sort of powerless to help on this one and I actually tried with a related PR :-), because I don't have access to the internal systems/logs. Any chance of investigation and merge on this one? Other than buck-related things this should work...

@mikehardy
Copy link
Contributor

@dulmandakh entropy is starting to take hold on this one, with a conflict

@fkgozali
Copy link
Contributor

fkgozali commented Jan 9, 2021

Sorry I was quite swamped with the new architecture work for RNTester, I’ll try to pick this up again early next week. Any help figuring out the Buck issue would be very useful as well.

@mikehardy
Copy link
Contributor

I wonder if the failure above will self-resolve after resolving conflicts and re-pushing here. I say that because I noticed "KitKat" in the error and I believe minSdkVersion is 21 now, KitKat is 19. One can hope

@fkgozali
Copy link
Contributor

fkgozali commented Jan 9, 2021

Yeah that’s possible, I was just thinking about that. @dulmandakh wanna try rebasing?

@dulmandakh
Copy link
Contributor Author

Got it, will do soon

@fkgozali
Copy link
Contributor

So I looked into this a bit. This PR was days before the commit to bump test to run SDK 21: 7dcecb2

So to save time, I'm going to merge this PR and we'll fix forward any breakage after.

@facebook-github-bot
Copy link
Contributor

@fkgozali merged this pull request in 280f524.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jan 14, 2021
@fkgozali
Copy link
Contributor

Side note: testing this locally would require installing https://github.com/react-native-community/docker-android image locally, which is quite a big set up.

@fkgozali
Copy link
Contributor

This seems to pass: https://app.circleci.com/pipelines/github/facebook/react-native/7775/workflows/1b8515a7-bb5b-44ce-a404-674d04b828da/jobs/184957

So we're all good to go.

@mikehardy
Copy link
Contributor

Just in time for fresco 2.4.0 :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants