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

New GRPC Build System #1

Merged
merged 37 commits into from
Nov 15, 2024
Merged

New GRPC Build System #1

merged 37 commits into from
Nov 15, 2024

Conversation

binaryoverload
Copy link
Collaborator

  • Added the buf build tool to create language bindings for protos
  • Added a GitHub Actions workflow to automatically build and publish protobufs
  • Added a PR GitHub Actions workflow to check linting, build success and breaking changes

What's left to do:

  • Create a NPM_TOKEN secret to allow publishing to npm

Copy link
Contributor

github-actions bot commented Nov 8, 2024

The latest Buf updates on your PR. Results from workflow Validate Pull Request / check_buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed⏩ skipped❌ failed (86)❌ failed (1)Nov 15, 2024, 9:59 AM

Copy link
Member

@jonbarrow jonbarrow left a comment

Choose a reason for hiding this comment

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

Thank you for this! Only a couple things:

  1. I mentioned this in the review itself, but the formatting for these files is not the same as our typical setup. Was the a reason for this?
  2. I know that the building has been moved to npm rather than make now (which is fine 👍) so it makes sense to keep the package.json in the root, but why is the Go files also in the root? It's not a HUGE deal, I'm just curious since it looks a bit odd to be mixing language files like this

.editorconfig Show resolved Hide resolved
.gitattributes Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/pull_request.yml Outdated Show resolved Hide resolved
buf.gen.yaml Show resolved Hide resolved
buf.gen.yaml Outdated Show resolved Hide resolved
Copy link
Collaborator

@mrjvs mrjvs left a comment

Choose a reason for hiding this comment

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

chatgpt approves this, so should be good

@binaryoverload binaryoverload merged commit bc7b864 into master Nov 15, 2024
0 of 2 checks passed
@binaryoverload binaryoverload deleted the monorepo branch November 15, 2024 17:05
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