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

Split the Echo example into model, implementation and runtime #567

Merged
merged 2 commits into from
Aug 28, 2019

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Aug 27, 2019

Motivation:

The generated code and implementation for the Echo example is used as
part of the example and as well as in the test suite. To make the
example more obvious we should split it into its constituent parts: the
model (i.e. the definition and generated code), the implementation (of
the server), and the runtime (CLI for client and server).

Modifications:

  • Split the Echo example into three modules: EchoModel (generated code),
    EchoImplementation (server implementation), and EchoRuntime (CLI for
    client and server).
  • Update the Makefile to target the updated paths
  • Add missing imports in tests
  • Make the generated code public (instead of internal) since we're
    no longer symlinking into the same module for tests

Result:

  • No more symlinks for Echo
  • Structure of example is more straightforward

Motivation:

The generated code and implementation for the Echo example is used as
part of the example and as well as in the test suite. To make the
example more obvious we should split it into its constituent parts: the
model (i.e. the definition and generated code), the implementation (of
the server), and the runtime (CLI for client and server).

Modifications:

- Split the Echo example into three modules: `EchoModel` (generated code),
  `EchoImplementation` (server implementation), and `EchoRuntime` (CLI for
  client and server).
- Update the `Makefile` to target the updated paths
- Add missing imports in tests
- Make the generated code `public` (instead of `internal`) since we're
  no longer symlinking into the same module for tests

Result:

- No more symlinks for Echo
- Structure of example is more straightforward
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 27, 2019

CLA Check
The committers are authorized under a signed CLA.

@glbrntt glbrntt requested a review from MrMage August 27, 2019 14:40
@MrMage
Copy link
Collaborator

MrMage commented Aug 28, 2019

Looks good, but do you think we could merge EchoModel and EchoImplementation? Shouldn't be much overhead, and saves us one extra target.

@glbrntt
Copy link
Collaborator Author

glbrntt commented Aug 28, 2019

I think it's better to keep them separate; it makes the separation of generation and implementation really obvious (i.e. for someone who's new to gRPC). WDYT?

@MrMage
Copy link
Collaborator

MrMage commented Aug 28, 2019

Fine with me :-)

@glbrntt glbrntt merged commit 705f085 into grpc:nio Aug 28, 2019
@glbrntt glbrntt deleted the echo-models branch August 28, 2019 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants