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

move json-rpc-engine from merged-packages to packages #1895

Merged
merged 15 commits into from
Nov 3, 2023

Conversation

kanthesha
Copy link
Contributor

@kanthesha kanthesha commented Oct 24, 2023

Explanation

In the process of migrating json-rpc-engine into core monorepo. In this PR we are moving json-rpc-engine from merged-packages to packages folder.

In this process, we followed below steps.
Moving to packages

  1. Move migration target from merged-packages/ to packages/.
  2. Run yarn install in the root directory.
  3. Check that all tests are passing in migration target by running yarn workspace @metamask/json-rpc-engine test.

Update downstream repos

  1. Bump the json-rpc-engine to 7.2.0 in root and packages
  2. Added references in tsconfig.json and tsconfig.build.json

Lints and Constraints

  1. Fixed yarn constraints errors
  2. Applied yarn workspace @metamask/json-rpc-engine changelog:validate

References

Changelog Changes

@metamask/eth-json-rpc-provider

  • CHANGED: Bump dependency on @metamask/json-rpc-engine to ^7.2.0

@metamask/json-rpc-engine

  • CHANGED: Bump dependency on @metamask/utils to ^8.2.0

@metamask/network-controller

  • CHANGED: Bump dependency on @metamask/json-rpc-engine to ^7.2.0

@metamask/permission-controller

  • CHANGED: Bump dependency on @metamask/json-rpc-engine to ^7.2.0

@metamask/queued-request-controller

  • CHANGED: Bump dependency on @metamask/json-rpc-engine to ^7.2.0

@metamask/selected-network-controller

  • CHANGED: Bump dependency on @metamask/json-rpc-engine to ^7.2.0

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate

@socket-security
Copy link

socket-security bot commented Oct 24, 2023

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: @types/jest@29.5.7

@socket-security
Copy link

socket-security bot commented Oct 24, 2023

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

@kanthesha kanthesha marked this pull request as ready for review October 24, 2023 22:38
@kanthesha kanthesha requested review from a team as code owners October 24, 2023 22:38
@kanthesha kanthesha self-assigned this Oct 24, 2023
@legobeat legobeat changed the title moving from merged-packages into packages move json-rpc-engine from merged-packages into packages Oct 25, 2023
@legobeat legobeat changed the title move json-rpc-engine from merged-packages into packages move json-rpc-engine from merged-packages to packages Oct 25, 2023
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

I've finished testing the tag porting script, and I think it will be safer to run it before the package is moved out of merged-packages/.

So #1802 will need to be merged before this PR can be merged. Sorry for the confusion!

MajorLift
MajorLift previously approved these changes Nov 3, 2023
Copy link
Contributor

@MajorLift MajorLift left a comment

Choose a reason for hiding this comment

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

Tags have been successfully ported to core!

git ls-remote --tags origin | grep 'json-rpc-engine'

@MajorLift
Copy link
Contributor

MajorLift commented Nov 3, 2023

Note: Some earlier release commits that were untagged in the original repo have been skipped by the tag porting script, and will be tagged manually.

  • TODO: GitHub may show these manually-created tags out of order due to sorting by tag creation date instead of release commit timestamp.

@MajorLift MajorLift requested a review from mcmire November 3, 2023 01:56
FrederikBolding
FrederikBolding previously approved these changes Nov 3, 2023
Copy link
Member

@FrederikBolding FrederikBolding left a comment

Choose a reason for hiding this comment

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

Reviewed PermissionController diff

"@metamask/rpc-errors": "^6.1.0",
"@metamask/utils": "^8.2.0",
Copy link
Member

Choose a reason for hiding this comment

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

Why is this package being downgraded?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for catching this! I just pushed a fix.

I'll add an entry in the migration checklist to double-check for dependency version bumps or other changes in main that may have happened while the PR is open.

For future migrations, we plan to coordinate to minimize the time these PRs are kept open so there's less chance of something like this slipping through.

@FrederikBolding FrederikBolding dismissed their stale review November 3, 2023 07:29

Found something

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

I updated the PR description to mention the dependency changes to other packages being made here. This looks good.

@mcmire mcmire merged commit c6ff1b9 into main Nov 3, 2023
119 checks passed
@mcmire mcmire deleted the move-json-rpc-engine-to-packages branch November 3, 2023 16:21
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.

Moving json-rpc-engine from merged-packages into the packages
4 participants