Skip to content

Conversation

unsuman
Copy link
Contributor

@unsuman unsuman commented Aug 24, 2025

Note

This is the first PR out of three PRs. Meging all closes #3769

This PR refactors to use types, constants and functions from the new pkg/limatype package instead of the previous pkg/store and pkg/limayaml packages. This improves code organisation and updates all relevant imports and usages throughout the codebase. The changes are extensive and touch many files, but are primarily focused on type migration rather than functional changes. These changes are made to avoid cycle import issues, making the code more maintainable and modular.

@unsuman unsuman force-pushed the refactor/create-limatype branch 3 times, most recently from c205106 to 1dfd4b5 Compare August 24, 2025 10:52
@unsuman unsuman marked this pull request as ready for review August 24, 2025 11:18
@AkihiroSuda
Copy link
Member

Lint is failing.
Should be easy to fix, but let us know if you need help.

VMTypes: vmTypes,
VMTypesEx: vmTypesEx,
GuestAgents: make(map[limayaml.Arch]GuestAgent),
ShellEnvBlock: envutil.GetDefaultBlockList(),
Copy link
Member

Choose a reason for hiding this comment

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

Where has it gone?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, my bad. It mistakenly got deleted in commit 5d9na56 🤕

Copy link
Member

Choose a reason for hiding this comment

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

Can you fix this? @unsuman

@AkihiroSuda AkihiroSuda added kind/refactoring Refactoring gsoc/2025 Google Summer of Code 2025 area/vmdrivers VM driver infrastructure labels Aug 24, 2025
@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Aug 24, 2025
@afbjorklund
Copy link
Member

afbjorklund commented Aug 24, 2025

As I mentioned on the chat, do we need to move everything out from limayaml and store just to fix the cycles?

Or can we move just the "problematic" ones that are needed by the drivers, so that one can still import limayaml

@unsuman unsuman force-pushed the refactor/create-limatype branch 2 times, most recently from 66574bd to 9e583cd Compare August 24, 2025 16:49
@unsuman
Copy link
Contributor Author

unsuman commented Aug 24, 2025

As I mentioned on the chat, do we need to move everything out from limayaml and store just to fix the cycles?

Or can we move just the "problematic" ones that are needed by the drivers, so that one can still import limayaml

Checked and I can say that not everything has been moved from the limayaml to limatype, i.e. only the problematic ones. We need to import the driverutil pkg in the store pkg, the import cycle issue is happening at this point.

[EDIT]: store -> driverutil -> driver -> store
*Here's a catch: I don't remember the exact issue that made me refactor the store/dirnames and store/filenames into limatype

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM, except the ShellEnvBlock regression

@unsuman unsuman force-pushed the refactor/create-limatype branch from 9e583cd to f07bae9 Compare August 25, 2025 09:45
Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 22af485 into lima-vm:master Aug 25, 2025
62 of 63 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/vmdrivers VM driver infrastructure gsoc/2025 Google Summer of Code 2025 kind/refactoring Refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor: Lima Config and External Drivers
3 participants