-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
Add system packages and an app for Go #663
Conversation
cmd/system/system.go
Outdated
Use: "system", | ||
Short: "System apps", | ||
Long: `Apps for systems.`, | ||
Aliases: []string{"v"}, |
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.
Why v
?
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 copied from the venafi app so we can change that to s
func UntarNested(r io.Reader, dir string) error { | ||
return untarNested(r, dir) | ||
} | ||
|
||
func untarNested(r io.Reader, dir string) (err error) { |
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.
Should we move this functions under pkg/archive/untar.go
?
I think it makes much more sense.
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.
No I don't see any reason to have one file for this.
command.RunE = func(cmd *cobra.Command, args []string) error { | ||
installPath, _ := cmd.Flags().GetString("path") | ||
version, _ := cmd.Flags().GetString("version") | ||
fmt.Printf("Installing Go to %s\n", installPath) |
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.
Should we introduce --quiet
flag as we did earlier for binary downloads?
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'm going to keep things verbose for the preview stages, to make it simpler / less work for everyone.
Then we can come back and fine tune.
Part of #654, tested on an Intel x86_64 machine. Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>
2ee281b
to
5a2392e
Compare
Signed-off-by: Alex Ellis (OpenFaaS Ltd) <[email protected]>
@Shikachuu do you want to have a go at the containerd recipe and prometheus next? @richardcase volunteered to take the firecracker CLI I didn't use a Go template in this Go app, but if we need it we can bring that back in for the additional apps we're working with. |
@alexellis sure I'll play around with containerd! |
Description
Add system packages and an app for Go
Motivation and Context
#654
How Has This Been Tested?
Types of changes
Documentation
I've updated the README
Checklist: