Output current rather than latest version in up-to-date messages #164
Conversation
Latest can actually be older than current, especially when comparing a snapshot against the latest release, and current is what was compared against latest anyway. While at it, add a test case for similar occurrence with Go versions (they were already showing correct info though).
WalkthroughThis update corrects the display of version information within the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
internal/goutil/goutil.go (3)
Line range hint
144-144: Consider making "unknown" a constant.The string "unknown" is used multiple times across the file, which could be made a constant to avoid repetition and potential errors in future modifications.
+ const unknownVersion = "unknown" - if current == "unknown" || available == "unknown" { + if current == unknownVersion || available == unknownVersion {
Line range hint
178-178: Refactor if-else chains to switch statements.Switch statements can improve readability and maintainability of the code by replacing multiple if-else conditions.
- if current == "unknown" || available == "unknown" { + switch { + case current == "unknown", available == "unknown":Also applies to: 203-203
Line range hint
24-24: Consider encapsulating global variables.Global variables can lead to issues with code maintainability and testing. Consider encapsulating these into a struct or providing getter/setter functions to control their access.
Also applies to: 26-26, 28-28, 30-30
internal/goutil/goutil_test.go (1)
Line range hint
167-167: Consider making "echo" a constant.The string "echo" is used multiple times, particularly in tests mocking the behavior of the
gocommand. Defining it as a constant would improve maintainability.+ const mockCommand = "echo" - goExe = "echo" + goExe = mockCommand
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- internal/goutil/goutil.go (2 hunks)
- internal/goutil/goutil_test.go (4 hunks)
Additional context used
golangci-lint
internal/goutil/goutil.go
144-144: string
unknownhas 7 occurrences, make it a constant (goconst)
178-178: ifElseChain: rewrite if-else to switch statement (gocritic)
203-203: ifElseChain: rewrite if-else to switch statement (gocritic)
24-24: goExe is a global variable (gochecknoglobals)
26-26: keyGoBin is a global variable (gochecknoglobals)
28-28: keyGoPath is a global variable (gochecknoglobals)
30-30: osMkdirTemp is a global variable (gochecknoglobals)
internal/goutil/goutil_test.go
549-549: Error return value of
os.Setenvis not checked (errcheck)
550-550: Error return value of
os.Setenvis not checked (errcheck)
167-167: string
echohas 3 occurrences, make it a constant (goconst)
Additional comments not posted (5)
internal/goutil/goutil.go (2)
81-81: Update to display the current version is correct.This change aligns with the PR's objective to display the current version instead of the latest when the package is up-to-date. This is particularly important for clarity and accuracy in version reporting.
99-99: Correct version information displayed.The modification ensures that the "Already up-to-date" message reflects the current version rather than the latest. This change is crucial for accuracy, especially in scenarios where the latest version might not necessarily be the most recent.
internal/goutil/goutil_test.go (3)
677-699: Good test coverage forCurrentToLatestStrfunction.This test ensures that the function correctly displays the "up-to-date" message with the current version. It's essential for verifying the changes made in the associated function.
725-747: Effective testing forVersionCheckResultStr.The test verifies that the function outputs the correct "up-to-date" message when the package version is current. This aligns well with the PR's goals and ensures the functionality behaves as expected.
773-795: Regex usage in test is appropriate.The use of regex to match the expected output string in
VersionCheckResultStris a robust method to ensure the correctness of the output, especially when dealing with dynamic values in strings.
|
@scop |
Latest can actually be older than current, especially when comparing a snapshot against the latest release, and current is what was compared against latest anyway.
While at it, add a test case for similar occurrence with Go versions (they were already showing correct info though).
Summary by CodeRabbit
Bug Fixes
Tests