-
Notifications
You must be signed in to change notification settings - Fork 302
Container: Add initial container for meshtastic-cli #847
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
CLA assistant check
All committers have signed the CLA.
44cb502 to
0531c91
Compare
29755a7 to
8e9d516
Compare
@ianmcorvidae
ianmcorvidae
left a comment
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.
One small comment, but in general I think this is good. I haven't actually tested the containerfiles myself but they look like they should work fine and hopefully you did, anyway!
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 don't use the v prefix for the tags in this repo -- https://github.com/meshtastic/python/tags
I imagine this should probably just build on any tag, or any three-part numeric thing that looks like a version number.
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.
good catch, I admit i didn't double check! I'll remove that and check.
And yes, I did test them as I actually needed the CLI without wanting to install everything :p
but the 'v*' is just a just an easy way to only do this (and publish) for tagged releases, I'll also double check if we can filter on 'digit(s).digit(s).digit(s) but I don't think so, as I would have probably added at least one digit after the 'v' so we could do
- '*.*.*' maybe?
.dockerignore
Outdated
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 can go either way on this, but if this is just for running the CLI itself, we probably don't need to worry about including the protobufs folder. It's really only used as an input for the update-protobufs script that builds actual python stuff in meshtastic/protobuf. If you think it's more useful to include it I don't have a problem with it though.
875b7de to
02c11a3
Compare
Just a quick set of files to enable the build of (tagged) containers. Both alpine and debian containers are available (~200MiB/~1.2GiB) allowing us to use meshtastic cli with a quick docker run, instead of having to build/install stuff locally. Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
02c11a3 to
77db301
Compare
Just a quick set of files to enable the build of (tagged) containers. Both alpine and debian containers are available (~200MiB/~1.2GiB) allowing us to use meshtastic cli with a quick docker run, instead of having to build/install stuff locally.