Skip to content

Conversation

@vax-r
Copy link
Contributor

@vax-r vax-r commented Oct 19, 2025

Summary

This PR includes the following 2 changes:

  • Fill provision.Script with default value ( empty string "" ) in FillDefault()
  • Make Probe.Script a pointer

Check #4211 for related details.

Notes

@jandubois -
This is suggested by you.
Would you mind to share your signed-off tag with me so I can put it in the commit as `Suggested-by: ?

vax-r added 2 commits October 19, 2025 17:57
Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
Change the type of "Script" under "type Probe struct"
from "string" to "*string". Make it consistent with
"Provision.Script".

Check lima-vm#4211 for details.

Signed-off-by: I Hsin Cheng <richard120310@gmail.com>
@jandubois
Copy link
Member

@jandubois -
This is suggested by you.
Would you mind to share your signed-off tag with me so I can put it in the commit as `Suggested-by: ?

My DCO line in this project is:

Signed-off-by: Jan Dubois <jan.dubois@suse.com>

But (in my opinion) it really is not necessary to put in Suggested-by: lines for things discussed in issues or code reviews.

}
}
if !strings.HasPrefix(p.Script, "#!") {
if p.Script != nil && !strings.HasPrefix(*p.Script, "#!") {
Copy link
Member

Choose a reason for hiding this comment

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

The Validate function does not have to worry about the Script field being nil because it has been set by FillDefaults. Same is true for the provision Script references.

Copy link
Member

Choose a reason for hiding this comment

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

Right, but doesn't seem to hurt checking it here too

@AkihiroSuda AkihiroSuda added this to the v2.0.0 milestone Oct 20, 2025
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 bc198f4 into lima-vm:master Oct 24, 2025
37 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