Skip to content

fix(java): tidy default released version - #6654

Closed
noahdietz wants to merge 3 commits into
googleapis:mainfrom
noahdietz:java-tidy-release-version
Closed

fix(java): tidy default released version#6654
noahdietz wants to merge 3 commits into
googleapis:mainfrom
noahdietz:java-tidy-release-version

Conversation

@noahdietz

Copy link
Copy Markdown
Contributor

Tidy the java.released_version if it was set to the default of 0.0.0.

Noticed this being serialized while testing a diff change.

@noahdietz
noahdietz requested a review from a team as a code owner July 6, 2026 21:50
@noahdietz
noahdietz requested a review from zhumin8 July 6, 2026 21:50
@noahdietz
noahdietz enabled auto-merge (squash) July 6, 2026 21:54

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds logic to clear the ReleasedVersion of a Java library if it matches a defaultReleasedVersion, and includes a unit test for this behavior. However, a review comment correctly points out that defaultReleasedVersion is undefined in the package, which will lead to a compilation error.

if library.Java.ReleasedVersion == "" {
return
}
if library.Java.ReleasedVersion == defaultReleasedVersion {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

critical

The variable defaultReleasedVersion is used here, but it is not defined anywhere in the java package or in internal/librarian/java/defaults.go. This will cause a compile-time error. Please define defaultReleasedVersion as a constant (e.g., "0.0.0") in this package.

@zhumin8

zhumin8 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

I think this better be part of add.
Based on the example, IIUC you are adding a new API path (google/cloud/kms/v2) to an existing library.
So I expect librarian add google/cloud/kms/v2 not changing released_version of the library. Note that the original config has released_version omitted because it can be derived from version: 2.97.0-SNAPSHOT, it should bereleased_version: 2.96.0 . released_version: 0.0.0 is not a right config.
Removing this via tidy is saying this is a valid config, only has redundant info.

@noahdietz

Copy link
Copy Markdown
Contributor Author

I think this better be part of add. Based on the example, IIUC you are adding a new API path (google/cloud/kms/v2) to an existing library. So I expect librarian add google/cloud/kms/v2 not changing released_version of the library. Note that the original config has released_version omitted because it can be derived from version: 2.97.0-SNAPSHOT, it should bereleased_version: 2.96.0 . released_version: 0.0.0 is not a right config. Removing this via tidy is saying this is a valid config, only has redundant info.

Thanks and your comment on the versions.txt PR clarified my confusion. Good catch. Closing this PR.

@noahdietz noahdietz closed this Jul 7, 2026
auto-merge was automatically disabled July 7, 2026 16:06

Pull request was closed

@noahdietz
noahdietz deleted the java-tidy-release-version branch July 7, 2026 16:06
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.

2 participants