Skip to content

fix(internal/librarian/golang): update version in snippet metadata#3945

Open
JoeWang1127 wants to merge 7 commits intomainfrom
fix/snippet-metadata
Open

fix(internal/librarian/golang): update version in snippet metadata#3945
JoeWang1127 wants to merge 7 commits intomainfrom
fix/snippet-metadata

Conversation

@JoeWang1127
Copy link
Contributor

@JoeWang1127 JoeWang1127 commented Feb 6, 2026

Replace placeholder with library version in snippet metadata file.

The logic is similar in go generator used in legacylibrarian:
https://github.com/googleapis/google-cloud-go/blob/b3409e6d7fbb5708e6be6000a1257a741c23b63b/internal/librariangen/module/module.go#L72

The file is read in one-go and modified in-place, which is not ideal if the file is too large. However, the metadata file is ~500 lines so this straightforward implementation should be suffice.

For #3617

Copy link
Contributor

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

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 introduces functionality to update a version placeholder in snippet metadata files. The implementation in generate.go uses filepath.Walk to find and update the files. A corresponding test is added in generate_test.go.

My review includes two main points:

  1. A suggestion to improve the performance of the file traversal by using filepath.WalkDir.
  2. A more significant concern with the new test TestUpdateSnippetMetadata, which is currently not testing the replacement logic correctly and modifies files in the testdata directory. I've provided a complete rewrite of the test to make it robust and self-contained, also noting that the suggested fix aligns with the rule of using t.Fatal for error handling in Go tests.

@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

❌ Patch coverage is 63.15789% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.29%. Comparing base (4e2fdfc) to head (e022cc7).

Files with missing lines Patch % Lines
internal/librarian/golang/generate.go 63.15% 4 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3945      +/-   ##
==========================================
- Coverage   83.35%   83.29%   -0.07%     
==========================================
  Files          69       69              
  Lines        6188     6207      +19     
==========================================
+ Hits         5158     5170      +12     
- Misses        671      675       +4     
- Partials      359      362       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JoeWang1127 JoeWang1127 marked this pull request as ready for review February 7, 2026 00:29
@JoeWang1127 JoeWang1127 requested a review from a team as a code owner February 7, 2026 00:29
@JoeWang1127 JoeWang1127 enabled auto-merge (squash) February 7, 2026 01:04
return cerr
}

func updateSnippetMetadata(library *config.Library, output string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

pull over the doc strings

also, I think we should start logging more things, similar to how legacy librarian does so.

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