refactor certigo package main #201
Conversation
| defer tty.Close() | ||
| } | ||
|
|
||
| tty.WriteString("Enter password") |
There was a problem hiding this comment.
Error return value of tty.WriteString is not checked (from errcheck)
|
|
||
| tty.WriteString("Enter password") | ||
| if prompt != "" { | ||
| tty.WriteString(fmt.Sprintf(" for entry [%s]", prompt)) |
There was a problem hiding this comment.
Error return value of tty.WriteString is not checked (from errcheck)
| if prompt != "" { | ||
| tty.WriteString(fmt.Sprintf(" for entry [%s]", prompt)) | ||
| } | ||
| tty.WriteString(": ") |
There was a problem hiding this comment.
Error return value of tty.WriteString is not checked (from errcheck)
| result.CertificateRequestInfo = cri | ||
| for _, cert := range connState.PeerCertificates { | ||
| if *connectPem { | ||
| pem.Encode(stdout, lib.EncodeX509ToPEM(cert, nil)) |
There was a problem hiding this comment.
Error return value of pem.Encode is not checked (from errcheck)
|
|
||
| func inputFiles(fileNames []string) ([]*os.File, error) { | ||
| var files []*os.File | ||
| if fileNames != nil { |
There was a problem hiding this comment.
S1031: unnecessary nil check around range (from gosimple)
| tty.SetDefaultPassword(*dumpPassword) | ||
| } | ||
|
|
||
| files, err := inputFiles(*dumpFiles) |
There was a problem hiding this comment.
SA4006: this value of err is never used (from staticcheck)
cli/cli.go
Outdated
|
|
||
| file, err := inputFile(*verifyFile) | ||
| if err != nil { | ||
| fmt.Fprintf(errOut, err.Error()) |
There was a problem hiding this comment.
SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (from staticcheck)
This splits the logic into a cli, which handles parsing command line args and reading files, and a terminal abstraction for handling user input and ouput. All uses of os.Exit are removed in favor of returning errors. Overall this enables better testing and reuse of code. Previously we had to rely on external unit testing for CLI tests, which are harder to write tests.
mbyczkowski
left a comment
There was a problem hiding this comment.
This seems like a good refactoring to me. golintci nits can be handled in a separate PRs. @mcpherrinm do you think it makes sense to merge #178 first and then rebase this PR or the other way around?
|
let's merge this and keep improving from there; I'm not sure #178 is ready for merge yet |
This splits the logic into a cli, which handles parsing command line args and
reading files, and a terminal abstraction for handling user input and ouput.
All uses of os.Exit are removed in favor of returning errors.
Overall this enables better testing and reuse of code. Previously we had to
rely on external unit testing for CLI tests, which are harder to write tests.