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

shell: More types than ever #21426

Merged
merged 8 commits into from
Jan 8, 2025
Merged

Conversation

mvollmer
Copy link
Member

@mvollmer mvollmer commented Dec 12, 2024

No description provided.

@mvollmer mvollmer added the blocked Don't land until something else happens first (see task list) label Dec 12, 2024
@mvollmer
Copy link
Member Author

This is still another approach, just rename everything (almost) and get it relaxed green.

pkg/shell/nav.tsx Fixed Show fixed Hide fixed
@martinpitt martinpitt removed the blocked Don't land until something else happens first (see task list) label Dec 18, 2024
@martinpitt
Copy link
Member

@mvollmer unblocked!

@mvollmer mvollmer force-pushed the shell-typing-4 branch 3 times, most recently from 7fb1850 to f9267bb Compare January 3, 2025 15:54
A CockpitNav component wants the "filtering" function to put a keyword
on each item, but the item can be anything otherwise. CockpitHosts
uses it with Machine objects, and PageNav with ManifestItems.
@mvollmer mvollmer marked this pull request as ready for review January 7, 2025 12:27
@mvollmer mvollmer requested a review from martinpitt January 7, 2025 12:28
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! Nothing too serious, but I think it needs one more cleanup round.

pkg/shell/machines/machines.d.ts Outdated Show resolved Hide resolved
pkg/shell/failures.tsx Show resolved Hide resolved
pkg/shell/hosts.tsx Outdated Show resolved Hide resolved
pkg/shell/hosts.tsx Outdated Show resolved Hide resolved
pkg/shell/shell.tsx Outdated Show resolved Hide resolved
pkg/shell/shell.tsx Outdated Show resolved Hide resolved
Copy link
Member

@martinpitt martinpitt left a comment

Choose a reason for hiding this comment

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

Thanks! 😎

useEvent(keys as unknown as cockpit.EventSource<cockpit.EventMap>, "changed");

if (!keys)
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

setUnlockKey(key.name);
/* Key needs to be unloaded, do that directly */
} else if (!enable && key.loaded) {
keys.unload(key).catch(ex => setDialogError(ex.message));
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

onClose={() => dialogResult.resolve()}
title={_("SSH keys")}
id="credentials-modal"
footer={<Button variant='secondary' onClick={() => dialogResult.resolve()}>{_("Close")}</Button>}
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

footer={<Button variant='secondary' onClick={() => dialogResult.resolve()}>{_("Close")}</Button>}
>
<Stack hasGutter>
{dialogError && <ModalError dialogError={dialogError} />}
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

{ title: _("Toggle") },
] }
rows={ Object.keys(keys.items).map((currentKeyId, index) => {
const currentKey = keys.items[currentKeyId] || { name: 'test' };
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

Comment on lines +71 to +72
onTroubleshoot = () => {},
onReconnect = () => {},
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 added lines are not executed by any test.

@@ -202,8 +235,8 @@ export class CockpitHosts extends React.Component {
</Tooltip>
</>}
/>;
const label = current_machine.label || "";
const user = current_machine.user || this.state.current_user;
const label = current_machine?.label || "";
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

Comment on lines +183 to +184
arg.then(function(...args) { console.log(...args) },
function(...args) { console.error(...args) });
Copy link
Contributor

Choose a reason for hiding this comment

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

These 2 added lines are not executed by any test.

if (typeof arg.stream == "function")
arg.stream(function() { console.log.apply(console, arguments) });
arg.stream(function(...args) { console.log(...args) });
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test.

@mvollmer mvollmer merged commit 21cd1b3 into cockpit-project:main Jan 8, 2025
85 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants