Add yaml decode function for machine info#25
Conversation
There was a problem hiding this comment.
Pull request overview
Adds YAML decoding support for MachineInfo and devices, while also renaming existing JSON decode entrypoints to make the format explicit.
Changes:
- Renamed
Decode→DecodeJSONacross machine and device decode APIs. - Added YAML decode paths:
machine.DecodeYAML,device.DecodeYAML, and per-busDecodeYAML. - Updated/extended tests to cover the new decode functions (including YAML machine info decoding).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/machine/machine_golden_test.go | Updates golden test to use DecodeJSON. |
| pkg/machine/device/usb/usb.go | Renames JSON decode function and adds YAML decode for USB devices. |
| pkg/machine/device/usb/usb_test.go | Updates tests to use DecodeJSON. |
| pkg/machine/device/pci/pci.go | Renames JSON decode function and adds YAML decode for PCI devices. |
| pkg/machine/device/pci/pci_test.go | Updates tests to use DecodeJSON. |
| pkg/machine/device/fastrpc/fastrpc.go | Renames JSON decode function and adds YAML decode for FastRPC devices. |
| pkg/machine/device/decode.go | Introduces DecodeJSON and new DecodeYAML dispatch by bus. |
| pkg/machine/device/decode_test.go | Updates tests to use DecodeJSON. |
| pkg/machine/decode.go | Introduces DecodeJSON and new DecodeYAML for machine info. |
| pkg/machine/decode_test.go | Updates JSON decode tests and adds YAML machine info decode tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
farshidtz
left a comment
There was a problem hiding this comment.
Thanks. The changes are fine, but I'm not sure why this is needed in order to keep the inference snaps CLI backward compatible. That feels like a requirement that must be satisfied on the CLI module, although that is currently in beta and undergoing major changes anyway.
|
Closing this as the team decided not to add YAML decoding support to lscompute at this stage. We are going to change |
Add YAML decoders for machine info and devices. This is required for backwards compatibility in the inference snap cli.