-
Notifications
You must be signed in to change notification settings - Fork 215
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
Improve check for installed git #171
Improve check for installed git #171
Conversation
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.
Also lefthook help
should be fixed also, however it looks like it is a built-in command.
cmd/root.go
Outdated
var fs = afero.NewOsFs() | ||
gitInitialized, _ := afero.DirExists(fs, filepath.Join(getRootPath(), ".git")) | ||
if !gitInitialized { | ||
log.Fatal("git is not initialized") |
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.
That message should be more verbose to give user a hint what to do. What git is not initialized
does mean, after all?
Maybe something like that?
log.Fatal("git is not initialized") | |
log.Fatal("This command must be executed within git repository. Change working directory or initialize new repository with `git init`.") |
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.
✅
7c1bad7
to
503c78d
Compare
503c78d
to
7f04086
Compare
@@ -43,6 +44,21 @@ var rootCmd = &cobra.Command{ | |||
Long: `After installation go to your project directory | |||
and execute the following command: | |||
lefthook install`, | |||
PersistentPreRun: func(cmd *cobra.Command, args []string) { | |||
if cmd.Name() == "help" || cmd.Name() == "version" { |
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.
this is a bit brittle but couldn't find a better way
@@ -115,8 +131,7 @@ func setRootPath(path string) { | |||
// get absolute path to .git dir (project root) | |||
cmd := exec.Command("git", "rev-parse", "--show-toplevel") | |||
|
|||
outputBytes, err := cmd.CombinedOutput() | |||
check(err) | |||
outputBytes, _ := cmd.CombinedOutput() |
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 check the error because we either checked it in the global PersistentPreRun
or don't care about git (i.e., in help
or version
)
Yeah it was a Work in progress PR, I guess it's ready now |
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
Closes #90, closes #114.