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

Remove proto/api submodule #5868

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Remove proto/api submodule #5868

wants to merge 8 commits into from

Conversation

dnr
Copy link
Member

@dnr dnr commented May 7, 2024

What changed?

  • Remove proto/api submodule.
  • Adds a new program that's automatically run by the Makefile as required to output the api protos in binary form, which can be used by most tools. The command is run by go run and uses the same api-go reference as the rest of the server for the source protos.
  • Removed http annotations from internal matchingservice proto.

Why?

  • Ensure that the api-go import (in go.mod) and the proto files imported by this repo's proto files are always in sync.
  • Simplify update process, stop having to mess with submodules.

How did you test it?

running commands manually a lot

Potential risks

Importing protos from somewhere other than our api and google well-known protos may require some updates to the script's import path logic.

Documentation

updated docs

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
Copy link
Contributor

@tdeebswihart tdeebswihart left a comment

Choose a reason for hiding this comment

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

Your protogen version is out of date as we pin protogen in the makefile; it's separate from the Go API version in go.mod

@dnr
Copy link
Member Author

dnr commented May 7, 2024

Your protogen version is out of date as we pin protogen in the makefile; it's separate from the Go API version in go.mod

I know, I'm waiting to do an api release so I can bump it

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.

None yet

2 participants