-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[featureflag] - use full demo protos #271
Conversation
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.
@puckpuck thanks! This looks good. All the extra modules is why I was waiting to do this -- and the fact the ffs_demo_pb
module is now huge. But looking at this PR it seems that new module is only 7k lines and the old 2k, so not bad and not an issue.
I would suggest using a symlink to link to the top level demo.proto
if possible -- I think Windows supports those now? -- otherwise I'll focus on making a change to the grpc server generator to support reading files from parent directories.
I created #279 to track updating Dockerfile for this service |
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.
lgtm
Unblocks #245
Changes
Uses a full copy of the
demo.proto
to build corresponding gRPC modules. Previously we were using a small subset ofdemo.proto
without thepackage hipstershop
declaration. There are several new files in this PR because we are now building the Erlang behavior modules for all the services part of the protobuf definition. This can be addressed in another PR to clean up the docker build process for this service.For significant contributions please make sure you have completed the following items:
Does this PR require a changelog update?
CHANGELOG.md
updated for non-trivial changes