[fix] Fix debug.ModuleVersion #3079
Conversation
Add tests, fix parsing issues and add a specical case for the main module. Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
There was a problem hiding this comment.
Hello @dominikschulz, 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!
Summary of Changes
This pull request addresses issues with the debug.ModuleVersion function in the gopass password manager. It includes fixes for parsing module versions, adds a special case for the main module, and incorporates new tests to ensure the function works as expected. The changes aim to improve the accuracy and reliability of version information displayed in logs.
Highlights
- Fixes for
debug.ModuleVersion: The pull request fixes parsing issues in thedebug.ModuleVersionfunction, ensuring correct version retrieval. - Special case for main module: A special case has been added to handle the main module's version, ensuring it's correctly identified and parsed.
- Added tests: New tests have been added to verify the functionality of
debug.ModuleVersion, increasing confidence in its correctness.
Changelog
Click here to see the changelog
- Makefile
- Added
-buildvcs=truetoBUILDFLAGS_NOPIEto ensure version control information is included in builds.
- Added
- helpers/modinfo/main.go
- Created a new helper program to print build info and module versions for debugging purposes.
- internal/backend/storage/fs/store.go
- Corrected the module path used by
debug.ModuleVersionin theVersionfunction togithub.com/gopasspw/gopass/internal/backend/storage/fs.
- Corrected the module path used by
- pkg/debug/version.go
- Modified
ModuleVersionto handle the main module's version and to gracefully handle invalid or missing module versions. - Added logging for when module versions cannot be parsed.
- Added a helper function
pathsto extract module paths for debugging purposes.
- Modified
- pkg/debug/version_test.go
- Created new tests for
ModuleVersionto cover various scenarios, including valid and invalid module versions, and build info failures.
- Created new tests for
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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.
A module's version, a tricky thing,
To parse and fetch, and make it sing.
With tests and fixes, we now ensure,
The correct version will endure.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request addresses issues with module versioning in gopass, adds tests, and handles special cases for the main module. The changes look good overall and improve the reliability of version information.
Summary of Findings
- Error Handling in Module Version Parsing: The code now gracefully handles errors during version parsing by logging the error and returning a semver.Version with build information. This prevents the application from crashing when encountering an invalid version string.
- Special Case for Main Module: The code includes a special case to handle the main module's version, ensuring that the correct version is returned even if it's not in the dependencies.
- Test Coverage: The addition of tests significantly improves the reliability of the
ModuleVersionfunction by covering various scenarios, including valid and invalid module versions, non-existent modules, and build info failures.
Merge Readiness
The pull request is in good shape and addresses the identified issues effectively. The added tests provide confidence in the correctness of the changes. I recommend merging this pull request after addressing the comments below. I am unable to directly approve this pull request, and other reviewers should review and approve this code before merging.
| if m == "github.com/gopasspw/gopass" || strings.HasPrefix(m, "github.com/gopasspw/gopass/") { | ||
| sv, err := semver.Parse(strings.TrimPrefix(bi.Main.Version, "v")) | ||
| if err == nil { | ||
| return sv | ||
| } | ||
| Log("Failed to parse version %q for %q (gopass): %s", bi.Main.Version, m, err) |
There was a problem hiding this comment.
This special case for gopass ensures that the main module's version is correctly parsed and returned. It handles cases where the version might not be in the dependencies list.
Signed-off-by: Dominik Schulz <dominik.schulz@gauner.org>
Add tests, fix parsing issues and add a specical case for the main module.
Logs looking much better now: