-
Notifications
You must be signed in to change notification settings - Fork 49
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
adding graph ql for Helm versions - master #22
adding graph ql for Helm versions - master #22
Conversation
); | ||
|
||
const releases = repository.releases.nodes.reverse(); | ||
let latestValidRelease = releases.find(release => isValidVersion(release.tagName, type)); |
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.
We expect latest release to be the latest version for the type?
This seems to be a behavior change compared to the older logic which allowed all releases to compete to be the latest:
responseArray.forEach(response => {
if (response && response.tag_name) {
let currentHelmVerison = semver.clean(response.tag_name.toString());
if (currentHelmVerison) {
if (currentHelmVerison.toString().indexOf('rc') == -1 && semver.gt(currentHelmVerison, latestHelmVersion)) {
//If current helm version is not a pre release and is greater than latest helm version
latestHelmVersion = currentHelmVerison;
}
}
}
});
Seems like a fair assumption to me. But I think this also presses the need for feature flagging to be on the safe side. Thoughts?
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.
Good point. But earlier we were comparing the versions so that we could get the last 3.* release of Helm. It's possible that they released a 2.* version after their latest Helm 3 release, and we didn't want to pick that up.
We can still assume that among all Helm 3.* releases, the last one would be the latest, and same goes for Helm 2.*.
Anyway, to be on the safe side I'll add a feature flag probably.
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.
Approving with suggestions.
Added a graphql query to fetch last 100 releases of helm, and then find the latest one (which is stable) based on user inputs.