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

[M2] app2 implementation #543

Merged
merged 44 commits into from
Sep 26, 2019
Merged

[M2] app2 implementation #543

merged 44 commits into from
Sep 26, 2019

Conversation

Darkren
Copy link

@Darkren Darkren commented Aug 30, 2019

Fixes #528

  • Added basic structures
  • Implemented methods for HSFrame

Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

📄

pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
@evanlinjin evanlinjin changed the title [WIP] app2 implementation [M2] [WIP] app2 implementation Sep 4, 2019
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

Good start.

pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/hsframe.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Show resolved Hide resolved
pkg/app2/server.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Outdated Show resolved Hide resolved
pkg/app2/server.go Outdated Show resolved Hide resolved
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

Beautifully written code.

Although there is some implementation queries. Specifically, we will need to support multiple networks in the future:

  • dmsg
  • skywire routes

My comments are mostly regarding this. Hence, you do not need to follow the exact comments, but please take some time to think of an optimal solution.

pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Show resolved Hide resolved
pkg/app2/manager.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
pkg/app2/server_rpc.go Outdated Show resolved Hide resolved
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

Clean code and good work so far. Please review my comments.

pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/listener.go Outdated Show resolved Hide resolved
pkg/app2/manager.go Outdated Show resolved Hide resolved
pkg/app2/network/dmsg_conn.go Outdated Show resolved Hide resolved
pkg/app2/rpc_gateway.go Outdated Show resolved Hide resolved
Copy link

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

Good stuff. Nearly there.

pkg/app2/network/addr.go Outdated Show resolved Hide resolved
pkg/app2/network/addr.go Outdated Show resolved Hide resolved
pkg/app2/rpc_client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
pkg/app2/client.go Outdated Show resolved Hide resolved
@Darkren Darkren changed the title [M2] [WIP] app2 implementation [M2] app2 implementation Sep 26, 2019
@Darkren Darkren merged commit 0ff83f9 into skycoin:mainnet-milestone2 Sep 26, 2019
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.

2 participants