Package Structure #11
Conversation
- Move main.go into cmd/example - Move vault into pkg/ - Create better configuration processing logic - Update tests accordingly - Refactor tests
| var dsv *vault.Vault | ||
|
|
||
| if config, err := vault.GetConfigFromEnv(); err != nil { | ||
| log.Fatalf("[ERROR] DSV_SDK_CONFIG or DSV_SDK_CONFIG_File must be set") |
There was a problem hiding this comment.
typo: "DSV_SDK_CONFIG_File" -> "DSV_SDK_CONFIG_FILE"
There was a problem hiding this comment.
error returned by GetConfigFromEnv() should not be ignored
| } | ||
|
|
||
| func ParseConfig(filePath string) (*Configuration, error) { | ||
| if content, err := ioutil.ReadFile(filePath); err != nil { |
There was a problem hiding this comment.
It is recommended to use os.ReadFile() in a new code. Please change to os.ReadFile(filePath)
| ConfigFileEnvVar = "DSV_SDK_CONFIG_FILE" | ||
| ) | ||
|
|
||
| func UnmarshalConfig(configJson []byte) (*Configuration, error) { |
There was a problem hiding this comment.
I think this func should not be exported.
| } | ||
| } | ||
|
|
||
| func ParseConfig(filePath string) (*Configuration, error) { |
There was a problem hiding this comment.
I think this func should not be exported.
| }) | ||
| t.Run("Delete", func(t *testing.T) { | ||
| if client, err := vault.Client(id); err != nil { | ||
| t.Errorf("calling clients.Client(\"%s\"): %s", id, err) |
There was a problem hiding this comment.
typo: "calling clients.Client(" -> "calling vault.Client("
There was a problem hiding this comment.
it is better to use %q instead of \"%s\"
| }) | ||
| t.Run("Get", func(t *testing.T) { | ||
| if client, err := vault.Client(id); err != nil { | ||
| t.Errorf("calling clients.Client(\"%s\"): %s", id, err) |
There was a problem hiding this comment.
typo: "calling clients.Client(" -> "calling vault.Client("
There was a problem hiding this comment.
it is better to use %q instead of \"%s\"
| client := &Client{clientResource: clientResource{RoleName: roleName}} | ||
|
|
||
| if err := vault.New(client); err != nil { | ||
| t.Errorf("calling clients.New(\"%s\"): %s", roleName, err) |
There was a problem hiding this comment.
typo: "calling clients.New(" -> "calling vault.New("
There was a problem hiding this comment.
it is better to use %q instead of \"%s\"
| } else if v := os.Getenv(ConfigFileEnvVar); v != "" { | ||
| return ParseConfig(v) | ||
| } else { | ||
| return nil, nil |
There was a problem hiding this comment.
probably should return non-nil error in this case
|
Thanks for your suggestions. I will accept them all, however, I'm sorry, but I mistakenly submitted this PR. I meant to mark it draft and should have submitted it as an issue first, anyway. I won't delete the PR now that the discussion is here but I have marked it as a draft. |
Adopt the standard cmd/, pkg/ structure and update the tests accordingly.