buf migrate to v2 #235
buf migrate to v2 #235
Conversation
|
|
kmrmt
force-pushed
the
feature/buf/migrate-to-v2
branch
from
ef6eb1a
to
d38cd7a
Compare
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to several configuration files related to the build and generation processes. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant buf.gen.yaml
participant buf.yaml
User->>Makefile: Trigger build
Makefile->>buf.gen.yaml: Read configuration
buf.gen.yaml->>Makefile: Provide updated plugin reference
Makefile->>buf.yaml: Read module and linting rules
buf.yaml->>Makefile: Provide linting rules
Makefile->>User: Build complete
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
buf.gen.yaml (1)
'buf' Command Not Found
The
generate_dependenciesoption requires thebuftool to be installed and properly configured. Please ensure thatbufis added as a project dependency and installed in your development environment.🔗 Analysis chain
Line range hint
7-17: Approve new 'generate_dependencies' option, verify its impact.The addition of the
generate_dependenciesoption is approved. This option likely instructs the plugin to generate code for dependencies as well, which can be beneficial for a more complete code generation process.To understand the full impact of this new option, please:
- Check the documentation of the
buf.build/community/timostamm-protobuf-tsplugin for thegenerate_dependenciesoption.- After running the code generation, verify the output:
Ensure that the generated code meets your project's requirements and doesn't introduce any unexpected side effects.
The
bufcommand is not found, which is required to utilize thegenerate_dependenciesoption effectively. Please ensure thatbufis installed and properly configured in your development environment.To proceed, you can add the following script to verify the presence and version of
buf:🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of generate_dependencies option buf generate # List generated files to see if dependencies are included find src -type f -name "*.ts" | sortLength of output: 1294
Script:
# #!/bin/bash # Check if buf is installed if command -v buf >/dev/null 2>&1; then echo "buf is installed. Version:" buf --version else echo "buf is not installed." fiLength of output: 69
buf.yaml (1)
17-19: Dependencies look good, but consider version pinning.The external dependencies are correctly declared and use official Protocol Buffer repositories:
- buf.build/bufbuild/protovalidate (likely for message validation)
- buf.build/googleapis/googleapis (common for Google API-related protobuf definitions)
However, the versions of these dependencies are not specified. To ensure reproducibility and avoid unexpected changes, consider pinning specific versions for each dependency.
Example of version pinning:
deps: - buf.build/bufbuild/protovalidate:v0.1.7 - buf.build/googleapis/googleapis:75e9812f2f0dReplace the version numbers with the appropriate ones for your project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
buf.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
- Makefile (0 hunks)
- buf.gen.yaml (1 hunks)
- buf.work.yaml (0 hunks)
- buf.yaml (1 hunks)
💤 Files with no reviewable changes (2)
- Makefile
- buf.work.yaml
🧰 Additional context used
🔇 Additional comments (6)
buf.gen.yaml (3)
Line range hint
1-17: Overall approval: buf.gen.yaml updated to v2 with appropriate changes.The changes in this file consistently reflect an upgrade to buf v2:
- Version updated to v2
- Plugin reference syntax updated to use
remote- New
generate_dependenciesoption addedThese changes appear well-structured and in line with buf v2 specifications. Ensure to test the new configuration thoroughly to verify that code generation works as expected with these updates.
1-1: Approve version update to v2, but verify compatibility.The update to version 2 of the buf configuration is a good practice to stay current. However, ensure that all other configurations and dependencies in your project are compatible with this new version.
To verify the compatibility, please run the following command and check for any warnings or errors:
5-5: Approve plugin reference change, verify v2 specification compliance.The change from
plugintoremotefor referencing the protobuf-ts plugin aligns with modern remote plugin fetching practices. This change is likely part of the v2 specification.To ensure this change complies with the v2 specification, please check the buf documentation or run the following command:
buf.yaml (3)
1-1: LGTM: Version declaration is correct.The version declaration for buf.yaml is correct and up-to-date, using the latest stable version (v2) of the buf tool.
2-16: Module configuration looks good, but verify exclusions.The module configuration for "vald" is well-structured and includes both linting and breaking change rules. However, please confirm that the following exclusions are intentional and necessary for your project:
Linting exclusions:
- FIELD_NOT_REQUIRED
- PACKAGE_NO_IMPORT_CYCLE
Breaking change exclusions:
- EXTENSION_NO_DELETE
- FIELD_SAME_DEFAULT
These exclusions may impact the strictness of your Protocol Buffer definitions and breaking change detection.
1-19: Overall, the buf.yaml configuration looks good.The configuration file is well-structured and follows buf.yaml conventions. It correctly sets up the buf tool for version v2, configures the "vald" module with appropriate linting and breaking change rules, and declares necessary dependencies.
To further improve the configuration:
- Verify that the linting and breaking change rule exclusions are necessary for your project.
- Consider pinning specific versions for the external dependencies to ensure reproducibility.
These improvements will enhance the maintainability and stability of your Protocol Buffer definitions.
vankichi
force-pushed
the
feature/buf/migrate-to-v2
branch
from
d38cd7a
to
c14f5d5
Compare
Signed-off-by: Kosuke Morimoto <[email protected]> Signed-off-by: vankichi <[email protected]>
vankichi
force-pushed
the
feature/buf/migrate-to-v2
branch
from
c14f5d5
to
392c8cd
Compare
Signed-off-by: Kosuke Morimoto <[email protected]> Signed-off-by: vankichi <[email protected]>
…migrate-to-v2 Backport: buf migrate to v2 (#235)
SSIA
Summary by CodeRabbit
New Features
buf.yaml, with versioning and module specifications.buf.yaml.Improvements
buf.gen.yamlto versionv2with a new method for plugin referencing.Bug Fixes
NODE_ROOTvariable in the Makefile.Chores
buf.work.yamlfile to streamline configuration management.