-
Notifications
You must be signed in to change notification settings - Fork 12
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
Helm: Allow users to view and use repositories from Helm Hub #463
Conversation
richard-cox
commented
Sep 1, 2020
•
edited
Loading
edited
- fixes Helm: Allow users to view and use repositories from Helm Hub #445
- To review
- Update register helm hub ui component
- need to add endpoint type specific actions to endpoints list view (helm sync)
- use for helm sync
- favourite cards are NOT by subtype, so parent type must have label info
- Fix display of disconected text & box in endpoint favourite card - Don't go out to fetch helm schema if there's none defined - Fixes following merge
I've updated following review comments. The Charts list repo filter --> side bar change is still pending |
@@ -51,14 +51,20 @@ export interface HelmInstallValues { | |||
releaseName: string; | |||
releaseNamespace: string; | |||
values: string; | |||
chart: string; | |||
chart: { |
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.
find common type (name/repo/version)
constructor( | ||
store: Store<any>, | ||
public helper: HelmReleaseHelperService, | ||
private route: ActivatedRoute, |
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.
route can go?
@@ -256,7 +256,7 @@ export class CreateReleaseComponent implements OnInit, OnDestroy { | |||
...this.details.value, | |||
...this.overrides.value, | |||
chart: { | |||
name: this.route.snapshot.params.name, | |||
chartName: this.route.snapshot.params.chartName, |
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.
change to name
@@ -62,7 +63,8 @@ export class UpgradeReleaseComponent { | |||
const name = chart.upgrade.name; | |||
const repoName = chart.upgrade.repo.name; | |||
const version = chart.release.chart.metadata.version; | |||
this.listConfig = new ReleaseUpgradeVersionsListConfig(store, repoName, name, version); | |||
this.listConfig = new ReleaseUpgradeVersionsListConfig(store, repoName, name, version, chart.monocularEndpointId); |
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.
const version = chart.release.chart.metadata.version; pattern
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.
LGTM