-
Notifications
You must be signed in to change notification settings - Fork 47
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
Added a new command to change version #185
Added a new command to change version #185
Conversation
Signed-off-by: Kartikay <[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.
LGTM.
pkg/cmd/update/update.go
Outdated
Long: ``, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
updateVersion := args[0] | ||
var homeDir = "/usr/local/bin/" |
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 need to specify some different path for windows. You can update this in windows switch case. You can create a custom directory for storing. Also it will be great if we can add this PATH to environment variables. We can use backslashes and the path will look maybe something like this var homeDir = "C:\\litmusctl\\or\\anything\\else\\you can mention"
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.
Hardcoding the homeDir is not the correct approach. You can use the package "github.com/mitchellh/go-homedir"
for finding the home directory for the executing user. This package is already being used in litmusctl
pkg/cmd/update/update.go
Outdated
Long: ``, | ||
Run: func(cmd *cobra.Command, args []string) { | ||
updateVersion := args[0] | ||
var homeDir = "/usr/local/bin/" |
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.
Hardcoding the homeDir is not the correct approach. You can use the package "github.com/mitchellh/go-homedir"
for finding the home directory for the executing user. This package is already being used in litmusctl
Signed-off-by: Kartikay <[email protected]>
homeDir, err := homedir.Dir() | ||
if err != nil { | ||
utils.PrintError(err) | ||
} |
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.
Hey @SarthakJain26, Thanks for your meaningful suggestion!, I have implemented the same here.
@kartikaysaxena the build pipeline is failing due to unused homeDir package name, this is not actually an issue, but as a fix you can replace |
Signed-off-by: Kartikay <[email protected]>
@@ -8,7 +8,7 @@ import ( | |||
"runtime" | |||
|
|||
"github.com/litmuschaos/litmusctl/pkg/utils" | |||
"github.com/mitchellh/go-homedir" | |||
homedir "github.com/mitchellh/go-homedir" |
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.
@SarthakJain26 I have implemented the same, kindly PTAL, thanks!
Added upgrade command to change version of the litmusctl binary, works like
sudo litmusctl update 1.1.0
, tested on linux arm64 architecture. It works for every version that exists, meaning not only it can upgrade but downgrade as well, but the command upgrade is already taken hence I chose update.Fixes #3307