build(internal/librarianops/upgrade): add librarian version fetcher function#3925
build(internal/librarianops/upgrade): add librarian version fetcher function#3925miguelvelezsa wants to merge 6 commits intoupgrade-commandfrom
Conversation
![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 function, GetLatestLibrarianVersion, to fetch the latest version of the librarian tool by executing a go list command, along with corresponding tests. The implementation is sound, but I have a couple of suggestions. My feedback includes a minor correction to remove a redundant type conversion and a more substantial recommendation to refactor the test for improved logic, robustness, and adherence to the repository's testing style guide.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## upgrade-command #3925 +/- ##
===================================================
+ Coverage 83.35% 83.36% +0.01%
===================================================
Files 69 70 +1
Lines 6188 6193 +5
===================================================
+ Hits 5158 5163 +5
Misses 671 671
Partials 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
internal/librarianops/upgrade/librarian_version_fetcher_test.go
Outdated
Show resolved
Hide resolved
| // get librarian version. The command is essentially a "deep dive" request for information about a | ||
| // specific Go module version. | ||
| func GetLatestLibrarianVersion(ctx context.Context) (string, error) { | ||
| version, err := command.Output(ctx, "go", "list", "-m", "-f", "{{.Version}}", "github.com/googleapis/librarian@main") |
There was a problem hiding this comment.
We have a version command, can we use that?
There was a problem hiding this comment.
I think this will depend of what we want, the librarian version seems to get it from the binary used at the time of running and the description in #3911 and current generate command get the latest librarian version (the go command I am using).
I think we want the one from binary, but worth to double confirm? @julieqiu
internal/librarianops/upgrade/librarian_version_fetcher_test.go
Outdated
Show resolved
Hide resolved
| // GetLatestLibrarianVersion run `go list -m -f '{{.Version}}' github.com/googleapis/librarian@main` to | ||
| // get librarian version. The command is essentially a "deep dive" request for information about a | ||
| // specific Go module version. | ||
| func GetLatestLibrarianVersion(ctx context.Context) (string, error) { |
There was a problem hiding this comment.
Put this in internal/librarianops/upgrade.go, there's no need for a separate package
There was a problem hiding this comment.
I am creating a package for the upgrade command (I can use librarianops). To be clear the file here is to make the code more maintainable :) now if we want to change how we get the librarian latest version we come to this file only and make the changes. Coders can easily knows what is happening, modify it and test it. For example in the upgrade.go file we know what the command is doing http://github.com/googleapis/librarian/blob/60048af3404553980e0dab385eee0ce78b7a4ec9/internal/librarianops/upgrade/upgrade.go#L27 we just have the implementation there.
Something different to the current generate command https://github.com/googleapis/librarian/blob/main/internal/librarianops/generate.go.
3 participants
[1 of 4]
Created librarian_version_fetcher file to create the function to get latest librarian version.
Function will be used in upgrade command for first step: #3911.