-
Notifications
You must be signed in to change notification settings - Fork 667
kubevirt: Add VM Dashboard Inventory Card #2751
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
kubevirt: Add VM Dashboard Inventory Card #2751
Conversation
|
cc @andybraren |
|
/retest |
|
Rebased to @rawagner, can you please have a look? |
|
Rebased as #2600 is merged. |
rawagner
left a comment
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.
just one comment, otherwise lgtm
| @@ -10,3 +11,6 @@ export const getVMIApiPath = (vmi: VMIKind) => | |||
| `${VirtualMachineInstanceModel.apiVersion}/namespaces/${getNamespace(vmi)}/${ | |||
| VirtualMachineInstanceModel.plural | |||
| }/${getName(vmi)}`; | |||
|
|
|||
| export const getVMINetworks = (vmi: VMIKind) => _.get(vmi, 'spec.networks', []); | |||
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.
please add return type ie VMIKind['spec]['networks'], same for getVMIDisks
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.
Per discussion in other PR, I have replaced the _.get() by direct access.
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.
nice
|
Rebased, #2601 is merged now |
|
/lgtm |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
tests are failing /hold |
On the VM Details Overview tab.
|
/retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mareklibra, rawagner The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold cancel |

On the VM Details Overview tab.
Depends on:
Just the last commit is relevant.