Skip to content
This repository was archived by the owner on Nov 9, 2023. It is now read-only.

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Oct 3, 2022

This is the release candidate for version 4.0.0.

github-actions and others added 2 commits October 3, 2022 17:00
@jpuri jpuri requested review from Gudahtt and mcmire October 3, 2022 17:37
@jpuri jpuri marked this pull request as ready for review October 3, 2022 17:37
@jpuri jpuri requested a review from a team as a code owner October 3, 2022 17:37
Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
@jpuri jpuri requested a review from mcmire October 4, 2022 11:48
@@ -6,9 +6,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [4.0.0] - 2022-10-03
### Changed
Copy link
Member

Choose a reason for hiding this comment

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

This PR is missing: #15

Specifically, the change in minimum supported Node.js version is notable, because it's a breaking change.

Copy link

Choose a reason for hiding this comment

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

Whoops, I missed this one. Might be good then to add something like:

- Add Node 12 as minimum required version [#15](https://github.com/MetaMask/json-rpc-middleware-stream/pull/15)

right before

- Retry pending requests when notification to reconnect is received ([#27](https://github.com/MetaMask/json-rpc-middleware-stream/pull/27))

?

Copy link
Member

Choose a reason for hiding this comment

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

Looks good, except it should be prefixed with "BREAKING:"

Copy link

Choose a reason for hiding this comment

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

Yeah, good point.

Copy link
Member

Choose a reason for hiding this comment

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

@jpuri could you add the "BREAKING:" prefix, so that people know which change is breaking?

Copy link
Contributor

Choose a reason for hiding this comment

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

PR is updated

@jpuri jpuri requested a review from Gudahtt October 4, 2022 17:46
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link

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

@jpuri jpuri merged commit c7d4a1a into main Oct 5, 2022
@jpuri jpuri deleted the release/4.0.0 branch October 5, 2022 05:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants