DNM #1236
DNM #1236
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds kubevirt to the list of providers considered for rd-network-kargs. The implementation for kubevirt is a stub that correctly indicates the feature is not yet supported by logging a warning. The changes are functionally correct. I've left a couple of minor comments regarding code style to improve readability and consistency.
As a general note, it would be helpful to use more descriptive titles and provide a brief description for pull requests in the future. This helps reviewers understand the context and purpose of the changes more quickly.
| use crate::providers::proxmoxve::ProxmoxVEConfigDrive; | ||
| use crate::providers::vmware::VmwareProvider; | ||
| use crate::providers::kubevirt::KubeVirtProvider; |
There was a problem hiding this comment.
For consistency and readability, it's good practice to keep use statements sorted alphabetically. The kubevirt import should be placed before proxmoxve to maintain alphabetical order. Running rustfmt would likely handle this automatically.
| match provider { | ||
| "vmware" => VmwareProvider::try_new()?.rd_network_kargs(), | ||
| "proxmoxve" => ProxmoxVEConfigDrive::try_new()?.rd_network_kargs(), | ||
| "kubevirt" => KubeVirtProvider::try_new()?.rd_network_kargs(), |
There was a problem hiding this comment.
To improve readability, it's helpful to keep match arms sorted alphabetically by their string patterns. The "kubevirt" arm should be placed before "proxmoxve" and "vmware". This is a minor style point, but it improves code consistency.
qinqon
force-pushed
the
kubevirt_rd_network_kargs
branch
6 times, most recently
from
cfe9a82 to
1f1cbba
Compare
Signed-off-by: Enrique Llorente <[email protected]>
qinqon
force-pushed
the
kubevirt_rd_network_kargs
branch
from
1f1cbba to
3c1f0aa
Compare
No description provided.