Skip to content

Update new_architecture.rb for React-ImageManager - #38247

Closed
billnbell wants to merge 2 commits into
react:mainfrom
billnbell:patch-2
Closed

Update new_architecture.rb for React-ImageManager#38247
billnbell wants to merge 2 commits into
react:mainfrom
billnbell:patch-2

Conversation

@billnbell

@billnbellbillnbell commented Jul 7, 2023

Copy link
Copy Markdown
Contributor

Summary:

This added React-ImageManager to the use_frameworks! - a lot of rpm modules podspec need this.

Changelog:

[iOS] [FIXED] - Add React-ImageManager path to work with use_frameworks!

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests

Test Plan:

Should not be breaking - it will add to the header_search_paths React-ImageManager

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Hi @billnbell!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@billnbell

Copy link
Copy Markdown
ContributorAuthor

I signed the CLA

@analysis-bot

analysis-bot commented Jul 7, 2023

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a8,885,449-2
androidhermesarmeabi-v7a7,935,798+1
androidhermesx869,281,970-1
androidhermesx86_649,185,456-1
androidjscarm64-v8a9,473,452+0
androidjscarmeabi-v7a8,416,807+0
androidjscx869,456,195-1
androidjscx86_649,772,407-1

Base commit: cbf9408
Branch: main

@facebook-github-bot

Copy link
Copy Markdown
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@facebook-github-botfacebook-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 Jul 7, 2023
@cipolleschi

Copy link
Copy Markdown
Contributor

Hi there! Thanks for taking the time for doing this.
I think we have also to update the ruby tests, otherwise the CI will stay red. I think this is the test that needs to be updated.

@github-actionsgithub-actionsBot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 19, 2023
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@billnbell

Copy link
Copy Markdown
ContributorAuthor

Hi there! Thanks for taking the time for doing this.
I think we have also to update the ruby tests, otherwise the CI will stay red. I think this is the test that needs to be updated.

Yeah I added that one - let's see what the tests do

@cortinicocortinico removed the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 20, 2023
@cipolleschi

Copy link
Copy Markdown
Contributor

/rebase

@github-actionsgithub-actionsBot added the Needs: Repro This issue could be improved with a clear list of steps to reproduce the issue. label Jul 20, 2023
@billnbell
billnbellforce-pushed the patch-2 branch 2 times, most recently from 9f2bd81 to 39f06ccCompareJuly 21, 2023 03:32
@billnbell

Copy link
Copy Markdown
ContributorAuthor

OK rebased.

@cipolleschi

Copy link
Copy Markdown
Contributor

I don't know what's going on with the CI. Main is green, I don't understand why this branch can't build correctly... :/

@cipolleschi

Copy link
Copy Markdown
Contributor

/rebase

@billnbell

Copy link
Copy Markdown
ContributorAuthor

What do you want me to do for /rebase ? I am not getting it.

@billnbell

Copy link
Copy Markdown
ContributorAuthor

I merged from Facebook into main and merged that and rebased into patch-2. Should be rebased.

@cipolleschi

Copy link
Copy Markdown
Contributor

Cool! Now the CI is running properly!

There are some ruby tests failures, which are expected!

You can run ruby tests locally on your machines:

  1. navigate to: react-native/packages/react-native/scripts
  2. run ./run_ruby_tests.sh

This will highlights the failures locally and you should be able to fix the tests and update the PR! ;)

Thanks again for taking the time to work on this!

@billnbell

Copy link
Copy Markdown
ContributorAuthor

ok fixed those 2.

@billnbell
billnbellforce-pushed the patch-2 branch 2 times, most recently from 51c9b3d to e1e1e76CompareJuly 29, 2023 04:48

@billnbellbillnbell left a comment

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

ok good

@cipolleschi

Copy link
Copy Markdown
Contributor

What do you want me to do for /rebase ? I am not getting it.

I'm sorry! 😓

Leaving a comment /rebase on a React Native PR triggers a github action that automatically rebase the PR on top of main, if possible.

That's why I was leaving those messages: I was asking the CI to rebase to rerun the CI and verify that it gets green!

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

fix: test for new arch
Squashed commit of the following:
commit dce55b0
Author: Bill Bell <bbell@exclusiveresorts.com>
Date: Wed Jul 19 21:48:27 2023 -0600
fix: test for new arch
commit 51e8515
Author: William Bell <williambell9708@outlook.com>
Date: Fri Jul 7 15:47:00 2023 -0600
Update new_architecture.rb for React-ImageManager
@github-actions

Copy link
Copy Markdown

This pull request was successfully merged by @billnbell in 8a63b91.

When will my fix make it into a release? | Upcoming Releases

@github-actionsgithub-actionsBot added the Merged This PR has been merged. label Jul 31, 2023
fortmarek pushed a commit that referenced this pull request Aug 8, 2023
Summary:
This added React-ImageManager to the use_frameworks! - a lot of rpm modules podspec need this.
bypass-github-export-checks
[iOS] [FIXED] - Add React-ImageManager path to work with use_frameworks!
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: #38247
Test Plan: Should not be breaking - it will add to the header_search_paths React-ImageManager
Reviewed By: dmytrorykun
Differential Revision: D47593749
Pulled By: cipolleschi
fbshipit-source-id: a66e90707e5fa73573deab1f04e8d8693869a90c
kelset pushed a commit that referenced this pull request Aug 11, 2023
Summary:
This added React-ImageManager to the use_frameworks! - a lot of rpm modules podspec need this.
bypass-github-export-checks
[iOS] [FIXED] - Add React-ImageManager path to work with use_frameworks!
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: #38247
Test Plan: Should not be breaking - it will add to the header_search_paths React-ImageManager
Reviewed By: dmytrorykun
Differential Revision: D47593749
Pulled By: cipolleschi
fbshipit-source-id: a66e90707e5fa73573deab1f04e8d8693869a90c
chore(releases): improve bump oss script to allow less human errors (#38666)
Summary:
One of the limitations of the existing flow for the release crew is that they need to manually remember to publish all the other packages in the monorepo ahead of a new patch release - this PR modifies the logic for the bump-oss-version script (and makes it available via yarn) so that it will not run if:
* there are git changes lying around
* if some of the packages need a new release
it required a bit of refactoring to extract some portions of the logic from the bump-all-package-versions script, but I think the end result is pretty decent.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[INTERNAL] [CHANGED] - improve bump oss script to allow less human errors
Pull Request resolved: #38666
Test Plan:
* checkout this branch
* comment L54 of bump-oss-version.js (to remove the check on the branch name)
* run `yarn bump-all-updated-packages`, verify that it works and that it detects that some packages have unreleased code
* run `yarn bump-oss-version -t asd -v asd` (the "fake" parameters are needed to pass the yargs check), verify that it will throw an error because it finds a package that has unreleased code
Reviewed By: mdvacca
Differential Revision: D48156963
Pulled By: cortinico
fbshipit-source-id: 2473ad5a84578c5236c905fd9aa9a88113fe8d22
# Conflicts:
#	scripts/publish-npm.js
re-add the file
nit
Revert "Update new_architecture.rb for React-ImageManager (#38247)"
This reverts commit 645264e249f566ce87f6aaf462e029909302fd92.
huntie pushed a commit that referenced this pull request Aug 18, 2023
…72 edition) (#38888)
Co-authored-by: William Bell <williambell9708@outlook.com>
resolved: #38247resolved: #38666
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Needs: ReproThis issue could be improved with a clear list of steps to reproduce the issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@billnbell@facebook-github-bot@analysis-bot@cipolleschi@cortinico@billnbell3