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

add protobuf files #557

Closed
wants to merge 1 commit into from
Closed

add protobuf files #557

wants to merge 1 commit into from

Conversation

colin-axner
Copy link
Contributor

No description provided.

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Do we want to put these files in separate directories from the specifications themselves, or include them alongside the specification documents?

@colin-axner
Copy link
Contributor Author

Do we want to put these files in separate directories from the specifications themselves, or include them alongside the specification documents?

I don't have a strong preference, but it'd be good to clearly document where to find the files if they don't live in proto/

cc @AdityaSripal

@colin-axner
Copy link
Contributor Author

I spoke with @AdityaSripal a little while back and we concluded that having the proto files live under proto/ made the most sense. The specs could link to the protobuf files as necessary.

I can add a README as well which talks about #568

I will probably split this pr into smaller ones so each review is more digestable. I'd like the protobuf files to be reviewed since they should contain up to date comments and no references to the SDK if they are to be included here. The only exception is the gogoproto can be left since it'd make our lives very difficult if they were removed

@cwgoes
Copy link
Contributor

cwgoes commented May 9, 2021

Thanks for the explanation, your proposal sounds good to me.

@AdityaSripal
Copy link
Member

Are you splitting these up @colin-axner or should I review as is?

@colin-axner
Copy link
Contributor Author

Are you splitting these up @colin-axner or should I review as is?

I should split these up. I think it'll be easier for all that way. This has fallen on my backlog (happy to let someone take over)

@colin-axner
Copy link
Contributor Author

I won't be taking this up anytime soon, so closing this pr

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.

3 participants