-
Notifications
You must be signed in to change notification settings - Fork 1
Protobuf update #21
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
base: main
Are you sure you want to change the base?
Protobuf update #21
Conversation
WalkthroughThis PR updates the project to support Protobuf 5.27.2 (from 3.21.12/4.21.12), migrates proto definitions from proto3 syntax to edition 2023 with implicit field presence annotations, and regenerates C++, C#, and Python compiled code. The fleet protocol version is bumped to 2.1.0. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@protobuf/README.md`:
- Around line 21-29: Change the incorrect heading level and remove the stray
orphan heading: rename the "#### Requirements" heading to "### Requirements" so
it follows the top-level "##" section, remove the extraneous "##" line after the
code block, and tidy surrounding blank lines to keep consistent Markdown
hierarchy in the README (look for the "#### Requirements" and the orphaned "##"
tokens to locate the edits).
🧹 Nitpick comments (3)
.gitignore (1)
4-5: Prefer directory-specific ignore for IDE cacheIf the intent is to ignore the
.ideadirectory only, consider.idea/for clarity and to avoid accidental matches.protobuf/README.md (1)
33-35: Consider adding language specifiers to code blocks.Adding
bashorshelllanguage identifiers to the fenced code blocks would improve syntax highlighting for readers.📝 Example fix
-``` +```bash find protobuf/definition -name "*.proto" -exec protoc -I=./protobuf/definition --cpp_out=./protobuf/compiled/cpp --csharp_out=./protobuf/compiled/cs --python_out=./protobuf/compiled/python --go_out=./protobuf/compiled/go/ --go_opt=paths=source_relative {} +Apply the same pattern to the other code blocks at lines 43, 49, 55, and 61. </details> </blockquote></details> <details> <summary>examples/protobuf_parsing_example/lib/protobuf/ProtobufModule.pb.h (1)</summary><blockquote> `2-4`: **Add CI check to verify protobuf generated code stays in sync with source `.proto` file.** Generated protobuf files (ProtobufModule.pb.h/.pb.cc) are checked into the repository but there is no automatic regeneration during build. Consider adding a CI step that regenerates the files from ExampleModule.proto and fails if any differences are detected. Additionally, document the regeneration command in the example's README (similar to the instructions in `protobuf/README.md` for C++, targeted at `examples/protobuf_parsing_example/lib/protobuf/`). </blockquote></details> </blockquote></details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| #### Requirements | ||
|
|
||
| - desired version of `protoc` installed ([prebuilt binaries](https://github.com/protocolbuffers/protobuf/releases)) | ||
| - `protoc-gen-go` needs to be installed to generate go files (set env value GOBIN to the path where protoc is installed first) | ||
| ```bash | ||
| go install google.golang.org/protobuf/cmd/protoc-gen-go@latest | ||
| ``` | ||
|
|
||
| ## |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix heading hierarchy and remove empty heading.
The #### Requirements heading skips heading levels (should be ### after ##), and line 29 has an orphaned ## that appears unintentional.
📝 Suggested fix
-#### Requirements
+### Requirements
- desired version of `protoc` installed ([prebuilt binaries](https://github.com/protocolbuffers/protobuf/releases))
- `protoc-gen-go` needs to be installed to generate go files (set env value GOBIN to the path where protoc is installed first)
```bash
go install google.golang.org/protobuf/cmd/protoc-gen-go@latest-##
Compile to all languages:
</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
21-21: Heading levels should only increment by one level at a time
Expected: h3; Actual: h4
(MD001, heading-increment)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @protobuf/README.md around lines 21 - 29, Change the incorrect heading level
and remove the stray orphan heading: rename the "#### Requirements" heading to
"### Requirements" so it follows the top-level "##" section, remove the
extraneous "##" line after the code block, and tidy surrounding blank lines to
keep consistent Markdown hierarchy in the README (look for the "####
Requirements" and the orphaned "##" tokens to locate the edits).
</details>
<!-- fingerprinting:phantom:medusa:ocelot -->
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.