Skip to content
This repository has been archived by the owner on Jun 12, 2023. It is now read-only.

move autogen grpc code into proto dep #1490

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jeffgrunewald
Copy link
Contributor

This change allows us to consume grpc client modules as application dependencies instead of using the grpcbox plugin to (re)generate them on the fly with each new application build regardless of whether or not the module definitions have changed upstream.

rebar.config Outdated
@@ -40,7 +41,7 @@
]}.

{plugins, [
{grpcbox_plugin, {git, "https://github.com/andymck/grpcbox_plugin.git", {branch, "andymck/ts-master"}}},
{grpcbox_plugin, {git, "https://github.com/andymck/grpcbox_plugin.git", {branch, "andymck/ts-master/combined-opts-and-template-changes"}}},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i'm not clear if this is even necessary anymore or if there's anything else that might be using it.

Copy link
Member

Choose a reason for hiding this comment

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

I've wondered about this.. did you try removing it?

Copy link
Contributor

Choose a reason for hiding this comment

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

grpcbox plugin is only used to generate the grpc PB files. If that has all moved out of miner then this is not required

@jeffgrunewald
Copy link
Contributor Author

if we move in this direction, this should be updated to point to the proto repo's master branch before merging

@jeffgrunewald jeffgrunewald force-pushed the jg/move_grpc_client_to_proto branch from afc9181 to ce54416 Compare June 10, 2022 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants