Conversation
Summary of ChangesHello @blakeli0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new Java showcase module designed to specifically validate the compatibility between the current protobuf runtime ( Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new module, java-showcase-3.25.8, to test compatibility between protobuf-java runtime v4.33.0 and gen code 3.25.8, including a README.md and pom.xml file. The reviewer suggests renaming the artifactId to java-showcase-3.25.8, updating the parent artifactId to java-showcase-3.25.8-parent, renaming the gapic-showcase.version property to something more descriptive like java-showcase.version, clarifying or updating the URL for compliance_suite.json, and explicitly defining the version for protobuf-java.
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>gapic-showcase</artifactId> |
There was a problem hiding this comment.
The artifactId for this new module should reflect its name, java-showcase-3.25.8, for clarity and consistency with the PR title. Currently, it's gapic-showcase.
| <artifactId>gapic-showcase</artifactId> | |
| <artifactId>java-showcase-3.25.8</artifactId> |
| </description> | ||
| <parent> | ||
| <groupId>com.google.cloud</groupId> | ||
| <artifactId>gapic-showcase-parent</artifactId> |
There was a problem hiding this comment.
The artifactId for the parent module should also be updated to reflect the new module's naming convention, e.g., java-showcase-3.25.8-parent or a more generic parent if applicable. Currently, it's gapic-showcase-parent.
| <artifactId>gapic-showcase-parent</artifactId> | |
| <artifactId>java-showcase-3.25.8-parent</artifactId> |
| </parent> | ||
|
|
||
| <properties> | ||
| <gapic-showcase.version>0.36.2</gapic-showcase.version> |
There was a problem hiding this comment.
The property name gapic-showcase.version might be misleading since this is a new module named java-showcase-3.25.8. Consider renaming it to something more descriptive like java-showcase.version or protobuf-gen-code.version to accurately reflect its purpose.
| <gapic-showcase.version>0.36.2</gapic-showcase.version> | |
| <java-showcase.version>0.36.2</java-showcase.version> |
| </goals> | ||
| <configuration> | ||
| <url> | ||
| https://raw.githubusercontent.com/googleapis/gapic-showcase/v${gapic-showcase.version}/server/services/compliance_suite.json |
There was a problem hiding this comment.
The URL for compliance_suite.json uses ${gapic-showcase.version}. If gapic-showcase.version refers to the original gapic-showcase module's version, this might be correct. However, if this new module has its own versioning for the compliance suite, this should be updated to use a relevant property (e.g., the one suggested in the previous comment). Please clarify or update this reference.
| https://raw.githubusercontent.com/googleapis/gapic-showcase/v${gapic-showcase.version}/server/services/compliance_suite.json | |
| https://raw.githubusercontent.com/googleapis/gapic-showcase/v${java-showcase.version}/server/services/compliance_suite.json |
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.protobuf</groupId> | ||
| <artifactId>protobuf-java</artifactId> |
There was a problem hiding this comment.
It's good practice to explicitly define the version for protobuf-java to ensure build reproducibility and avoid potential conflicts with other dependencies. Please add a <version> tag here.
<artifactId>protobuf-java</artifactId>
<version>3.25.8</version>
Add a new module java-showcase-3.25.8. This is a copy of the java-showcase module with a different version (3.25.8) of protobuf gen code. This is intended to test the compatibility between protobuf runtime v4.33.x and gen code 3.25.8. Add [a new step](https://github.com/googleapis/sdk-platform-java/blob/bccc9ded96f5313c22cd4543c25e49fb673792f6/.github/workflows/ci.yaml#L306-L312) in CI to test this module.
2 participants
Add a new module java-showcase-3.25.8. This is a copy of the java-showcase module with a different version (3.25.8) of protobuf gen code. This is intended to test the compatibility between protobuf runtime v4.33.x and gen code 3.25.8.
Add a new step in CI to test this module.