feat: use git config to read tsa server and include-certs #64
Conversation
bufferoverflow
force-pushed
the
feat/use-gitconfig
branch
from
b1c6ec4
to
2c7e910
Compare
|
@mastahyeti WDYT? It adds an additional dependency but people can get rid of wrappers. |
|
@mastahyeti no longer actively works on this project. I can take a look at this later this week. |
bufferoverflow
force-pushed
the
feat/use-gitconfig
branch
from
2c7e910
to
541cfc5
Compare
| @@ -72,6 +73,26 @@ func runCommand() error { | |||
| return nil | |||
| } | |||
|
|
|||
| // read tsa and include-certs from gitconfig | |||
| path, err := os.Getwd() | |||
| if err == nil { | |||
There was a problem hiding this comment.
Is there a reason we want to "fail open" here and not return if an error is returned?
There was a problem hiding this comment.
read tsa and include-cert should be optional, so just use the defaults if these are not defined within git config
| path, err := os.Getwd() | ||
| if err == nil { | ||
| repo, err := git.OpenRepository(path) | ||
| if err == nil { |
There was a problem hiding this comment.
Same question about failing open. The idiom used throughout is err != nil and returning an error. Is the idea here that we may not be in a repo and hence shouldn't fail? I'm not sure if the libgit2 bindings have helpers for this..but if we determine we aren't in a repo and/or there is no .gitconfig for that repo, wouldn't the most idiomatic thing be to check the user global and then system .gitconfig instead?
| if err == nil { | ||
| config, err := repo.Config() | ||
|
|
||
| tsa, err := config.LookupString("gpg.x509.smimesign.timestamp-authority") |
There was a problem hiding this comment.
What do we get from LookupString if the config value isn't set? Do we get an empty string (i.e. we match the default value)?
| } | ||
|
|
||
| includeCerts32, err := config.LookupInt32("gpg.x509.smimesign.include-certs") | ||
| if err == nil { |
There was a problem hiding this comment.
Same question here..if no such setting is set..do we get 0 here?
| ## Standalone usage | ||
|
|
||
| ```sh | ||
| $ smimesign --help |
Read configuration parameters
timestamp-authorityandinclude-certsfrom.gitconfigas discussed within #47