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 protogen build tool #139

Merged
merged 9 commits into from
Dec 8, 2023
Merged

Add protogen build tool #139

merged 9 commits into from
Dec 8, 2023

Conversation

tdeebswihart
Copy link
Contributor

What changed?
I've added the protogen build tool to manage compiling and post-processing protobuf-generated code.

Why?

We do a lot of work to post-process protobuf-generated code for our purposes and it's not sustainably to rewrite and maintain it separately for each repo when we're always doing the same thing.

This tool combines all of the following into a single build tool:

  • proto compilation
  • enum post-processing (renaming enum consts and rewriting String methods)
  • stripping plugin versions from the generated files
    • Alpine's version of proto is a full major version behind brew's, and some of our repos will fail CI if you differ from the alpine-generated code

How did you test it?
This tool is used in the PR stacked on this one that rebuilds the code as well as TODO in the server repo. I expect to add it to the CDS and saas-temporal repos as well once it's merged.

Potential risks

None. If I've broken something in another repo I'll fix it once I add it to that repo

We do a lot of work to post-process protobuf-generated code for our
purposes and it's not sustainably to rewrite and maintain it separately
for each repo when we're always doing the same thing.

This tool combines all of the following into a single build tool:

- proto compilation
- enum post-processing (renaming enum consts and rewriting String
  methods)
- stripping plugin versions from the generated files
  - Alpine's version of proto is a full major version behind brew's, and
    some of our repos will fail CI if you differ from the
    alpine-generated code
cmd/protogen/main.go Outdated Show resolved Hide resolved
cmd/protogen/main.go Outdated Show resolved Hide resolved
Good night, sweet prince.
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

👍 Looks like a good, simple approach. (I think if string manip instead of AST could simplify more, but this LGTM)

@tdeebswihart tdeebswihart merged commit eb1b5f4 into master Dec 8, 2023
4 checks passed
@tdeebswihart tdeebswihart deleted the tds/protoc-build-tool branch December 8, 2023 19:55
tdeebswihart added a commit to temporalio/temporal that referenced this pull request Dec 11, 2023
## What changed?
I replaced protoc.sh with
[protogen](temporalio/api-go#139)

## Why?
I've copied some variant of this protoc.sh tool into three repos now.
This is unsustainable, so I've added a little build tool to api-go that
will handle proto generation and post-processing for us.

## How did you test it?
[I ran `make proto` in the PR stacked on this
one](#5211)

## Potential risks
None

## Is hotfix candidate?
No
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants