-
-
Notifications
You must be signed in to change notification settings - Fork 343
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
refactor!: move getNetworkIdentity to network class #5453
Conversation
Performance Report✔️ no performance regression detected Full benchmark results
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!! Just the ? about the unit test moving somewhere instead of getting deleted
@@ -43,36 +41,6 @@ describe("node api implementation", function () { | |||
sinon.stub(networkStub, "localMultiaddrs").get(() => [multiaddr("/ip4/127.0.0.1/tcp/36000")]); | |||
}); | |||
|
|||
describe("getNetworkIdentity", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this unit test need to be reimplemented?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API impl now just calls a function, a unit test for it would be pretty stupid, testing that function a calls function b
🎉 This PR is included in v1.9.0 🎉 |
Motivation
Description
Move getNetworkIdentity logic into network class to reduce interface surface