-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: add a small warning for outdated CLIs #3454
Conversation
if !isSemver(version) || !isSemver(Version) { | ||
// if either version is not semver, we can't compare them | ||
// do a basic strict compare | ||
|
||
versionMatch = version == Version | ||
} else { | ||
|
||
serverVersion, err := semver.NewVersion(version) | ||
if err != nil { | ||
return result + fmt.Sprintf(` | ||
Server: Failed to parse the server version - %s`, err.Error()), false | ||
} | ||
|
||
cliVersion, err := semver.NewVersion(Version) | ||
if err != nil { | ||
return result + fmt.Sprintf(` | ||
Failed to parse the CLI version - %s`, err.Error()), false | ||
} | ||
|
||
versionConstrait, err := semver.NewConstraint(fmt.Sprintf(">=%d.%d", cliVersion.Major(), cliVersion.Minor())) | ||
if err != nil { | ||
return result + fmt.Sprintf(` | ||
Failed to parse the CLI version constraint - %s`, err.Error()), false | ||
} | ||
|
||
versionMatch = versionConstrait.Check(serverVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't change this logic (if you see any change, block the PR). I only detect if the CLI version if smaller than the server version and if so, show the outdated warning
return false, false, fmt.Errorf("failed to parse the CLI version - %w", err) | ||
} | ||
|
||
versionConstrait, err := semver.NewConstraint(fmt.Sprintf(">=%d.%d", cliSemVer.Major(), cliSemVer.Minor())) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
versionConstrait, err := semver.NewConstraint(fmt.Sprintf(">=%d.%d", cliSemVer.Major(), cliSemVer.Minor())) | |
versionConstraint, err := semver.NewConstraint(fmt.Sprintf(">=%d.%d", cliSemVer.Major(), cliSemVer.Minor())) |
This PR adds a small
(outdated)
text next to the user CLI version in case there's a new version available (using the server version as reference)Checklist
Loom video
Add your loom video here if your work can be visualized