-
Notifications
You must be signed in to change notification settings - Fork 693
fix limactl run bug by treating unparseable versions as latest instead of crashing during template validation. #3820
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
Conversation
134232b
to
f2abf5c
Compare
can someone review this? thanks @afbjorklund @jandubois |
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.
Thanks, LGTM
However, some comment changes and an additional test should be added.
Also might be a good idea to update all the GitHub actions that do a checkout with check-depth: 0
(see #3495 (comment)). That should not be necessary after this change, otherwise there is something wrong with it.
f2abf5c
to
d7155aa
Compare
Please resolve merge conflicts by rebasing on latest |
d04237f
to
d65ae91
Compare
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.
Thanks, LGTM
I see we have a bunch of checkout
actions that explicitly specify fetch-depth: 1
, which is redundant because it is the default. I wonder if those were added to explicitly show that we don't need the tags? I feel like they could all be removed, but should probably be done in a separate PR.
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.
Looks like the colima test is now failing: https://github.com/lima-vm/lima/actions/runs/17002343354/job/48209754473?pr=3820
+ colima start
time="2025-08-16T03:19:41Z" level=fatal msg="lima compatibility error: invalid semver version for Lima: d65ae91 is not in dotted-tri format"
I think it might be because it is because of this.
Let me remove it and see what happens. |
d65ae91
to
084e31b
Compare
Yes, I had the same thought too but I didn't want to touch them since the linked issue explicitly mentioned just |
No, it is not. It is a check in So for the |
084e31b
to
e72cac5
Compare
…d of crashing during template validation. Signed-off-by: olalekan odukoya <[email protected]>
e72cac5
to
c492c69
Compare
fixes issue