feat: use git config to read tsa server and include-certs#64
feat: use git config to read tsa server and include-certs#64bufferoverflow wants to merge 4 commits intogithub:mainfrom
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
ptoomey3
left a comment
ptoomey3
left a comment
There was a problem hiding this comment.
Initial set of comments/questions/etc. Also, thoughts on the extra needed to get something wired up to be able to test any of this?
|
|
||
| // 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 |
4 participants
Read configuration parameters
timestamp-authorityandinclude-certsfrom.gitconfigas discussed within #47