-
Notifications
You must be signed in to change notification settings - Fork 54
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
Support CSV output, error handling #42
Conversation
gauth.go
Outdated
cfgPath := os.Getenv("GAUTH_CONFIG") | ||
if cfgPath == "" { | ||
user, err := user.Current() | ||
usr, err := user.Current() |
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.
Just a minor cosmetic thing: If you're going to bother abbreviating anyway, might as well just use u
. It doesn't escape this scope anyway.
gauth.go
Outdated
cw := csv.NewWriter(os.Stdout) | ||
for _, record := range cfg { | ||
name, secret := record[0], record[1] | ||
_, curr, next, err := gauth.Codes(secret, currentTS) |
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.
Is there some reason to omit the previous code in the CSV output? If the goal is to make the output of the tool machine-readable, I'd suggest it should contain the same information as the human-readable 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.
Included.
gauth.go
Outdated
@@ -17,13 +19,16 @@ import ( | |||
) | |||
|
|||
func main() { | |||
useCSV := flag.Bool("csv", false, "Output CSV for easy machine processing") |
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.
A minor cosmetic point:
This is functionally correct, but it's fairly conventional to put flag definitions at global scope (done during initialization) so they are visible to the whole main package:
var (
useCSV = flag.Bool("csv", false, "Output CSV for easy machine processing")
)
func main() {
flag.Parse()
// …
}
There's only one right now, and it's only used inside main
at the moment, but based on some of the other open issues I predict there will be more flags coming along—and it'd be nice if they could be accessed from other helpers in the main package.
YMMV.
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.
Oh thanks for the reminder, haven't written go or touched that code in a long time, 💯 must fix.
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.
Done.
a43ce65
to
ca0967d
Compare
CSV output as suggested in #34.