Skip to content

chore: rename hermes-inspector-msggen to @react-native/hermes-inspector-msggen - #34850

Closed
gabrieldonadel wants to merge 5 commits into
react:mainfrom
gabrieldonadel:feat/move-hermes-msggen
Closed

chore: rename hermes-inspector-msggen to @react-native/hermes-inspector-msggen#34850
gabrieldonadel wants to merge 5 commits into
react:mainfrom
gabrieldonadel:feat/move-hermes-msggen

Conversation

@gabrieldonadel

Copy link
Copy Markdown
Collaborator

Summary

This PR renames hermes-inspector-msggen to @react-native/hermes-inspector-msggen, moves it to packages/hermes-inspector-msggen and makes the package private as part of RFC480.

Related to #34692

Changelog

[General] [Changed] - Move and rename hermes-inspector-msggen to @react-native/hermes-inspector-msggen

Test Plan

run sh ReactCommon/hermes/inspector/tools/run_msggen

@facebook-github-botfacebook-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. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Oct 3, 2022

@cortinicocortinico 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.

Can you remove the yarn.lock file?

@gabrieldonadel

Copy link
Copy Markdown
CollaboratorAuthor

Can you remove the yarn.lock file?

Sure, done

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.

Can we pushd and popd here as the paths are so distant?

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Sure, I've just pushed a commit updating this

@analysis-bot

analysis-bot commented Oct 3, 2022

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a7,739,362-17
androidhermesarmeabi-v7a7,142,028-27
androidhermesx868,050,472+8
androidhermesx86_648,022,301-25
androidjscarm64-v8a9,607,549+57
androidjscarmeabi-v7a8,373,071+70
androidjscx869,555,218+64
androidjscx86_6410,147,821+61

Base commit: 8486b4c
Branch: main

@cortinicocortinico 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.

Can you address the comments and rebase?

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.

You don't need || exit as you have a set -e on top

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Fixed

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.

I would suggest we pushd "${DIR}/msggen" (or $ROOT/packages/hermes-inspector-msggen) and popd at the end of this script so the user is back in the folder where the script was invoked.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

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

Sure, updated

@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@analysis-bot

analysis-bot commented Oct 4, 2022

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
ios-universaln/a--

Base commit: 108c876
Branch: main

@cortinicocortinico added the Tech: Monorepo For PRs that are related to the monorepo infra label Oct 4, 2022
@cortinicocortinico mentioned this pull request Oct 4, 2022
11 tasks
Comment threadpackages/hermes-inspector-msggen/package.json Outdated
@facebook-github-bot

Copy link
Copy Markdown
Contributor

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

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @gabrieldonadel in 530dae8.

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

@react-native-botreact-native-bot added the Merged This PR has been merged. label Oct 5, 2022
@gabrieldonadel
gabrieldonadel deleted the feat/move-hermes-msggen branch October 5, 2022 11:58
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.Shared with MetaApplied via automation to indicate that an Issue or Pull Request has been shared with the team.Tech: MonorepoFor PRs that are related to the monorepo infra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@gabrieldonadel@analysis-bot@facebook-github-bot@react-native-bot@cortinico