Skip to content
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

Add workload 'kind' param to 'show-details' event #7464

Merged
merged 4 commits into from
Apr 4, 2023

Conversation

aleksfront
Copy link
Contributor

@aleksfront aleksfront commented Apr 3, 2023

Note: showing details for Helm Charts and Releases are not affected. Sending custom events are necessary here.

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
@aleksfront aleksfront added enhancement New feature or request area/telemetry labels Apr 3, 2023
@aleksfront aleksfront requested review from jweak and ixrock April 3, 2023 10:15
@aleksfront aleksfront requested a review from a team as a code owner April 3, 2023 10:15
jweak
jweak previously approved these changes Apr 3, 2023
Copy link
Contributor

@jweak jweak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
Signed-off-by: Alex Andreev <alex.andreev.email@gmail.com>
@aleksfront aleksfront added this to the 6.5.0 milestone Apr 3, 2023
@aleksfront aleksfront requested a review from ixrock April 3, 2023 13:45

const terminal = ["create-terminal-tab"];

export type WhiteListItem =
| string
| { id: string; getParams: (...args: unknown[]) => AppEvent["params"] };
| { id: string; getParams: (...args: any[]) => AppEvent["params"] };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically unknown[] is more correct since the id could be wrong. But that is probably of low concern here. There are other improvements we could make in other places in the future.

return {
kind: parseKubeApi(selfLink as string).resource,
kind: selfLink ? parseKubeApi(selfLink).resource : "",
Copy link
Contributor

@ixrock ixrock Apr 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: What happens if selfLink exists but would be just incorrect? e.g. parseKubeApi("/404-error")?
I think parseKubeApi(selfLink)?.resource ?? "" would be more safe in this case, but probably not critical.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case parseKubeApi throws an error and event fires with error field.

@aleksfront aleksfront removed this from the 6.5.0 milestone Apr 4, 2023
@aleksfront aleksfront merged commit 2cf8a2f into master Apr 4, 2023
@aleksfront aleksfront deleted the add-workload-type-param-to-show-details-event branch April 4, 2023 06:59
@aleksfront aleksfront added this to the 6.5.0 milestone Apr 4, 2023
@Nokel81 Nokel81 mentioned this pull request Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/telemetry enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants