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

moving json-rpc-middleware-stream from merged-packages to actual pack… #1762

Merged
merged 43 commits into from
Nov 7, 2023

Conversation

kanthesha
Copy link
Contributor

@kanthesha kanthesha commented Oct 3, 2023

Explanation

We chose to place json-rpc-middleware-stream initially in the merged-packages and not packages so we can hide it from Yarn, ESLint, Prettier, and TypeScript. In this PR, we cleaned it up and moved it from merged-packages to actual 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-middleware-stream test.

Lints and Constraints

  1. Fixed yarn lint errors
  2. Fixed yarn constraints errors
  3. Applied yarn workspace @metamask/json-rpc-middleware-stream changelog:validate

References

Changelog Updates

@metamask/json-rpc-middleware-stream

  • BREAKING: Rename package from json-rpc-middleware-stream to @metamask/json-rpc-middleware-stream

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 3, 2023

@socket-security
Copy link

socket-security bot commented Oct 3, 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 force-pushed the json-rpc-middleware-stream-to-core-packages branch from ea8975c to f7ae183 Compare October 4, 2023 09:15
@mcmire
Copy link
Contributor

mcmire commented Oct 23, 2023

Per standup, we should take a look at the migration checklist for this package again as it may have changed since this PR was created. If there are any PRs we need to merge before this one can go in then we should take care of that first.

@kanthesha kanthesha self-assigned this Oct 23, 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 force-pushed the json-rpc-middleware-stream-to-core-packages branch 3 times, most recently from 284bc66 to c1423be Compare November 6, 2023 20:51
@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch 2 times, most recently from 504f5ef to a04e8b0 Compare November 6, 2023 21:17
@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch from a04e8b0 to 52ebe29 Compare November 6, 2023 21:26
@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch 2 times, most recently from 3596602 to a985575 Compare November 6, 2023 22:02
@MajorLift
Copy link
Contributor

MajorLift commented Nov 6, 2023

  • Suppressed @typescript-eslint/no-implied-eval lint error. Unable to repro on local:
    • Lint rule guards against strings being passed into setTimeout().
    • context.end is not a string type.
    • Not fixed by adding null checks or type guards (e.g. typeof context.end === 'function', typeof context.end !== 'string').
  • Likely a false positive due to this issue: Resolve discrepancy between yarn lint outputs on CI and local #1989

@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch 2 times, most recently from cf74a67 to 7f5e3b5 Compare November 6, 2023 22:23
@MajorLift MajorLift marked this pull request as ready for review November 6, 2023 22:34
@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch from 2e628b7 to 28782b2 Compare November 7, 2023 15:45
@MajorLift MajorLift force-pushed the json-rpc-middleware-stream-to-core-packages branch from 2bc0106 to 58e7d0c Compare November 7, 2023 16:58
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.

LGTM.

@MajorLift MajorLift merged commit c674b84 into main Nov 7, 2023
124 checks passed
@MajorLift MajorLift deleted the json-rpc-middleware-stream-to-core-packages branch November 7, 2023 18:33
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-middleware-stream from merged-packages into packages
4 participants