-
Notifications
You must be signed in to change notification settings - Fork 78
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
Check if name or email are present on global level before adding to git locally #106
base: master
Are you sure you want to change the base?
Conversation
…obal or system level
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.
Looks like the right approach overall :)
var stdout bytes.Buffer | ||
cmd := exec.Command("git", gitArgs...) | ||
cmd.Stdout = &stdout | ||
cmd.Stderr = ioutil.Discard |
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 seems redundant — not setting cmd.Stderr has the same effect, no?
gitArgs := append([]string{"config", "--get", "--null"}, args...) | ||
var stdout bytes.Buffer | ||
cmd := exec.Command("git", gitArgs...) | ||
cmd.Stdout = &stdout |
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.
Instead of dealing with the buffer manually, use cmd.Output()
instead of cmd.Run()
below.
if exitError, ok := err.(*exec.ExitError); ok { | ||
if waitStatus, ok := exitError.Sys().(syscall.WaitStatus); ok { | ||
if waitStatus.ExitStatus() == 1 { | ||
return "", &ErrNotFound{Key: args[len(args)-1]} |
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 have the ErrNotFound type, so just return err here
@@ -279,6 +279,26 @@ func runGitCommandIn(dir string, arg ...string) error { | |||
return cmd.Run() | |||
} | |||
|
|||
func execGitConfig(args ...string) (string, error) { | |||
gitArgs := append([]string{"config", "--get", "--null"}, args...) |
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.
Can you add a comment as to why --null is required here?
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.
Yes, I will. Per git-config
manpage, it "allows for secure parsing of the output without getting confused e.g. by values that contain line breaks." Thank you!
I think adding the name and email at the local level is a good thing, so I have reservations about merging this PR. What does everyone else think? |
@anthonyfok Hi, do you really do that for any of your other Git repos? What if your email address changes? |
f0abee6
to
989837a
Compare
Untested, but is it approximately what you had in mind? Thank you!