-
Notifications
You must be signed in to change notification settings - Fork 669
Add optional message to be shown to template #472
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
I like it!
|
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
HostArch string | ||
LimaHome string | ||
IdentityFile string | ||
} |
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.
I think we have to consolidate this struct into what we have for sockets
Lines 197 to 206 in 7d3eca5
data := map[string]string{ | |
"Dir": instDir, | |
"Home": home, | |
"Name": filepath.Base(instDir), | |
"UID": user.Uid, | |
"User": user.Username, | |
"Instance": filepath.Base(instDir), // DEPRECATED, use `{{.Name}}` | |
"LimaHome": limaHome, // DEPRECATED, (use `Dir` instead of `{{.LimaHome}}/{{.Instance}}` | |
} |
Having two similar but different structs is confusing.
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.
We consolidated the one from "limactl list", but didn't do anything about the socket templates
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, but I think we have to consolidate the template struct into what we already have for sockets
https://github.com/lima-vm/lima/pull/472/files#r767601007
I don't think that works too well. For For If we want to consolidate, then we can only do this for |
Turns out |
I found another issue with architecture, it was
I didn't end up using it now, but thought it might come in handy for instance when downloading a client... |
Can you point to where it is used as I would say that I don't think we need to include the GOARCH values as well; you can use templating to translate, if you really have to:
We can always add more names later, if they seem to be useful, but we can't remove them without breaking compatibility, so I would not add more than we need. I'm fine with |
It wasn't used per se, I just happened to list both in the same template test... But maybe should use the lima function to set them, instead of the "raw" vars like
|
Signed-off-by: Anders F Björklund <[email protected]>
Show example with current values already in place. Currently just shown as regular INFO log messages. Signed-off-by: Anders F Björklund <[email protected]>
On macOS and Windows, "podman-remote" is called "podman". On Linux, make sure that it calls the $CONTAINER_HOST... Signed-off-by: Anders F Björklund <[email protected]>
Signed-off-by: Anders F Björklund <[email protected]>
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
Show example with current values already in place.
Currently just shown as regular INFO log messages.
As mentioned in #470 (comment)
Example: