Skip to content

Back out "fix(iOS): Replace uses of CGColorRef with UIColor to avoid manual memory management"#46967

Closed
joevilches wants to merge 1 commit into
facebook:mainfrom
joevilches:export-D64194385
Closed

Back out "fix(iOS): Replace uses of CGColorRef with UIColor to avoid manual memory management"#46967
joevilches wants to merge 1 commit into
facebook:mainfrom
joevilches:export-D64194385

Conversation

@joevilches
Copy link
Copy Markdown
Contributor

Summary:
Original commit changeset: 5e85e17567e3

Original Phabricator Diff: D63989547

This is causing some crashes internally

Changelog: [Internal]

Differential Revision: D64194385

…d manual memory management"

Summary:
Original commit changeset: 5e85e17567e3

Original Phabricator Diff: D63989547

This is causing some crashes internally

Changelog: [Internal]

Differential Revision: D64194385
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 10, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request was exported from Phabricator. Differential Revision: D64194385

@joevilches
Copy link
Copy Markdown
Contributor Author

cc @Saadnajmi this was causing some problems internally. Not your code, I had to change some things that called RCTCreateCGColorRefFromSharedColor internally and that was the source of the issue. I will look into fixing that and bring this back afterwards

@Saadnajmi
Copy link
Copy Markdown
Contributor

cc @Saadnajmi this was causing some problems internally. Not your code, I had to change some things that called RCTCreateCGColorRefFromSharedColor internally and that was the source of the issue. I will look into fixing that and bring this back afterwards

I see, in the future, I guess I can separate removing RCTCreateCGColorRefFromSharedColor() to a different PR to try and limit issues like that?

@joevilches
Copy link
Copy Markdown
Contributor Author

Yeah that would at least make it so that you are not blocked. We could try doing that now if you don't want to be held up. There was only one location where this happened, but it is not immediately obvious to me how to reorganize it to avoid said problem

@Saadnajmi
Copy link
Copy Markdown
Contributor

Yeah that would at least make it so that you are not blocked. We could try doing that now if you don't want to be held up. There was only one location where this happened, but it is not immediately obvious to me how to reorganize it to avoid said problem

I would appreciate just bringing back the one method and not backing out the change if possible :D

@joevilches
Copy link
Copy Markdown
Contributor Author

That's a good idea! This issue is currently blocking some build infra and we are a few hours into landing, so I am gonna just wait for that. But I can bring it back while leaving that method in place for you. Should be easy enough

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Oct 10, 2024
@facebook-github-bot
Copy link
Copy Markdown
Contributor

This pull request has been merged in b0e2cb3.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @joevilches in b0e2cb3

When will my fix make it into a release? | How to file a pick request?

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. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants