Skip to content

build(internal/librarianops/upgrade): add version config file updater#3926

Open
miguelvelezsa wants to merge 5 commits intoupgrade-version-fetcherfrom
upgrade-config-updater
Open

build(internal/librarianops/upgrade): add version config file updater#3926
miguelvelezsa wants to merge 5 commits intoupgrade-version-fetcherfrom
upgrade-config-updater

Conversation

@miguelvelezsa
Copy link

[2 of 4]

Create librarian_config_updater file. File consist in the function to update a librarian.yaml config file.
Second step in #3911.

@miguelvelezsa miguelvelezsa requested a review from a team as a code owner February 6, 2026 03:18
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 new functionality to update the version in a librarian.yaml file, along with corresponding tests. The implementation is straightforward and the tests are well-written, covering the main success and failure scenarios. I have provided a suggestion to improve the code's clarity by correcting a function comment to accurately reflect its behavior.

miguelvelezsa and others added 4 commits February 5, 2026 19:21
@codecov
Copy link

codecov bot commented Feb 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.87%. Comparing base (bbe1e2c) to head (cf3e6f1).

Additional details and impacted files
@@                     Coverage Diff                     @@
##           upgrade-version-fetcher    #3926      +/-   ##
===========================================================
+ Coverage                    81.84%   81.87%   +0.03%     
===========================================================
  Files                           72       73       +1     
  Lines                         6273     6285      +12     
===========================================================
+ Hits                          5134     5146      +12     
  Misses                         791      791              
  Partials                       348      348              

☔ 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.


// UpdateLibrarianVersion updates the version field in the librarian.yaml config file with the provided version.
// If the file does not exist, returns an error.
func UpdateLibrarianVersion(version, configPath 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.

I think you can just use repoPath as an input and remove GenerateLibrarianConfigPath and getConfigFile, since they seem very straightforward.

Copy link
Member

Choose a reason for hiding this comment

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

func updateLibrarianVersion(version, repoDir string) error {
already exists, so you can reuse that

}
}

func TestGetConfigFile(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@@ -0,0 +1,49 @@
// Copyright 2026 Google LLC
Copy link
Member

Choose a reason for hiding this comment

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

ditto re: put this in internal/librarianops/upgrade.go as opposed to a separate package

t.Fatalf("failed to write initial config file: %v", err)
}

t.Run("Success", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

Make these two separate tests

TestUpdateLibrarianVersion and TestUpdateLibrarianVersion_Error

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.

3 participants