Skip to content
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 output for error handling #2

Merged

Conversation

antonlindstrom
Copy link
Contributor

This adds output for errors, previously it was fmt.Sprintf which will only
format and doesn't output the string.

The commit also provides more information about when the command isn't run in
the git toplevel. This could be solved by using git rev-parse --show-toplevel but as far as I can tell, this isn't supported in go-git
right now.

Super useful utility, thank you for creating this!

This adds output for errors, previously it was `fmt.Sprintf` which will only
format and doesn't output the string.

The commit also provides more information about when the command isn't run in
the git toplevel. This could be solved by using `git rev-parse
--show-toplevel` but as far as I can tell, this isn't supported in go-git
right now.
Copy link
Owner

@gonzaloserrano gonzaloserrano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR! I think the error printing change is good but the .git check is not needed. If you agree, could you remove that change from the PR?

gitbr.go Outdated

if os.IsNotExist(err) {
return nil, fmt.Errorf("no .git directory detected (needs to be in repository toplevel).")
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already covered by

fs := osfs.New(path)
fi, err := fs.Stat(".git")
in the PlainOpen call so we don't need to do it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, must have missed that. Thanks!

os.Exit(1)
}

err = uiRunner.Run()
if err != nil {
fmt.Sprintf("Error: %s", err.Error())
fmt.Printf("Error: %s\n", err.Error())
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

This removes a check that for the .git directory which is also done in go-git.
@gonzaloserrano gonzaloserrano merged commit f115754 into gonzaloserrano:master Jun 4, 2017
@gonzaloserrano
Copy link
Owner

Merged! 👍

@antonlindstrom antonlindstrom deleted the bugfix/error-handling branch June 4, 2017 14:32
@antonlindstrom
Copy link
Contributor Author

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants