-
Notifications
You must be signed in to change notification settings - Fork 161
Add version and revision info to Helm detail pages #2783
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
Conversation
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.
Works as expected, though I also put one suggestion for UI clarification.
ui/components/HelmChartDetail.tsx
Outdated
@@ -34,6 +34,8 @@ function HelmChartDetail({ className, helmChart }: Props) { | |||
info={[ | |||
["Type", Kind.HelmChart], | |||
["Chart", helmChart.chart], | |||
["Version", helmChart.version], | |||
["Revision", helmChart.revision], |
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.
💡 The "version" vs "revision" distinction is weird to me. At least to me, "version" and "revision" can be used interchangeably, and I'd use revision for "automatic" versioning (i.e. git commits) and version for "active" versioning (i.e. git tags) - and that's not at all what's going on here! Version is a "version mask", "version selector" or "version specification", and revision is "actual version" or "highest version matching the version field". I looked at the flux documentation to see if they use more precise terms, but the best I can find is "artifact revision", which is perfectly accurate but also requires you to have a deep understanding of the source controller for it to be helpful.
This isn't too bad on the helm release page (because "last applied" and "last attempted" helps explain that they're "discovered" and thus status, so that helps suggest "chart version" isn't discovered and thus specified), but here I can't help go "wait, what?".
tl;dr: My concrete suggestion is to change "Revision" to "Latest Revision" or "Current Revision". That way it would also "look like" it comes from status, to help distinguish between the two.
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.
"Current Revision" it is!
0c4f2a4
to
fc45a72
Compare
Closes #2706
The HelmChart class has two new getters which are displayed in the details view - Version, and Revision (should this be artifact revision?)
The HelmRelease class has three - Version, Last Applied Revision, and Last Attempted Revision
Screenshots below - yes, there's a couple unfortunate bugs I found here with the overflow styling of chip group...and a chip will be created if you just press enter on the search input with no content...........issue incoming :(