Add component information in app and service lists #796
Conversation
| @@ -20,7 +20,7 @@ class AppListItem extends React.Component<IAppListItemProps> { | |||
| link={`/apps/ns/${app.namespace}/${app.releaseName}`} | |||
| title={app.releaseName} | |||
| icon={icon} | |||
| info={app.version || "-"} | |||
| info={`${app.chart}/v${app.version || "-"}`} | |||
There was a problem hiding this comment.
the / version is confusing, for a second I thought it was repo/name. Maybe we could have a space between these instead?
There was a problem hiding this comment.
or maybe colon which seems more widespread? Alternative I'd go for the version in parenthesis
There was a problem hiding this comment.
Why do we need -? ${app.version || "-"}
There was a problem hiding this comment.
I think we should avoid confusing this with the chart version, the Helm CLI uses {chart.name}-{chart.version}. IMO a space will be the least confusing.
There was a problem hiding this comment.
Why do we need
-?${app.version || "-"}
That is because the version is not mandatory in some times that will be empty.
IMO a space will be the least confusing.
I'll go with the space then
There was a problem hiding this comment.
Actually I think that the separation between the two items using just a space is not very visible, let me try using two different lines
There was a problem hiding this comment.
nvm, I don't like the result of printing them as a column:
| @@ -20,7 +20,7 @@ class AppListItem extends React.Component<IAppListItemProps> { | |||
| link={`/apps/ns/${app.namespace}/${app.releaseName}`} | |||
| title={app.releaseName} | |||
| icon={icon} | |||
| info={app.version || "-"} | |||
| info={`${app.chart}/v${app.version || "-"}`} | |||
There was a problem hiding this comment.
Why do we need -? ${app.version || "-"}
| @@ -45,6 +45,7 @@ const ServiceInstanceCardList: React.SFC<IServiceInstanceCardListProps> = props | |||
| namespace={instance.metadata.namespace} | |||
| link={link} | |||
| icon={icon} | |||
| serviceClassName={(svcClass && svcClass.spec.externalName) || "-"} | |||
There was a problem hiding this comment.
wondering if to be safe you should also check svcClass.spec
There was a problem hiding this comment.
If a svcClass is found the spec is mandatory so it's not needed to check if it exists
Fixes #764
Add the chart name to the app list and the service name in the service instance list:
Note that changes in tiller-proxy were required in order to return the chart name in the list overview.