This repository has been archived by the owner on Apr 24, 2023. It is now read-only.
refactor cli of the smtp-service #10
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
February 17, 2020 10:29
refactor the cli of the smtp-service to have a structure more similar to the default cobra generated file structure. along with this, other changes have been made: - output a secret on the 'refresh' command - allow the secret name to be overridden on 'create' and 'refresh' - change the default output secret name to 'redhat-rhmi-smtp' - change the debug flow to be a single 'debug' boolean flag verification: - for each subcommand: 'create', 'get', 'refresh', 'delete', 'version', do the following steps - run the command, ensure it succeeds - run 'create' and 'refresh' with the 'secret-name' flag set to a different string and ensure the output secret name is overridden - run any command with the 'debug' flag set to 'true' and ensure that debug and info logs are shown and they're output to stderr
from bf3f9ec the 'refresh' subcommand will now return smtpdetails rather than a string with a new api key. this change in interface resulted in tests failing. this change updates the tests to not expect a string to be returned and to expect a smtpdetails struct instead. verification: - run 'make test/unit' - ensure all tests complete successfully
aidenkeating
force-pushed
the
refactor-cli
branch
from
1d680ea to
c2e5ad4
Compare
|
@dimitraz @austinmartinh Mind taking a look? This is a minor refactor of the CLI to give it some more structure and to ensure the secret name can be overridden in |
pkg/sendgrid/sendgridapi.go
Outdated
| } | ||
| } | ||
| if foundUser == nil { | ||
| return nil, &NotExistError{Message: fmt.Sprintf("uaer with username %s not found in sendgrid subuser list", username)} |
There was a problem hiding this comment.
Suggested change
| return nil, &NotExistError{Message: fmt.Sprintf("uaer with username %s not found in sendgrid subuser list", username)} | |
| return nil, &NotExistError{Message: fmt.Sprintf("user with username %s not found in sendgrid subuser list", username)} |
cmd/cli/version.go
Outdated
Comment on lines
35
to
43
| // Here you will define your flags and configuration settings. | ||
|
|
||
| // Cobra supports Persistent Flags which will work for this command | ||
| // and all subcommands, e.g.: | ||
| // versionCmd.PersistentFlags().String("foo", "", "A help for foo") | ||
|
|
||
| // Cobra supports local flags which will only run when this command | ||
| // is called directly, e.g.: | ||
| // versionCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") |
There was a problem hiding this comment.
Suggested change
| // Here you will define your flags and configuration settings. | |
| // Cobra supports Persistent Flags which will work for this command | |
| // and all subcommands, e.g.: | |
| // versionCmd.PersistentFlags().String("foo", "", "A help for foo") | |
| // Cobra supports local flags which will only run when this command | |
| // is called directly, e.g.: | |
| // versionCmd.Flags().BoolP("toggle", "t", false, "Help message for toggle") |
Should this comment block be removed?
added 3 commits
February 19, 2020 09:34
remove comments generated by the cobra cli tool, these include: - license headers - flag example comments license headers can be added to all files if needed in a later commit.
result of 'make vendor/fix'
aidenkeating
force-pushed
the
refactor-cli
branch
from
c2e5ad4 to
332648c
Compare
|
@austinmartinh I think I hit all those comments, mind taking one more look? |
aidenkeating
commented
Feb 19, 2020
pkg/sendgrid/sendgridapi.go
Outdated
| @@ -173,7 +173,7 @@ func (c *BackendAPIClient) ListSubUsers(query map[string]string) ([]*SubUser, er | |||
| query = map[string]string{} | |||
| } | |||
| listReq := c.restClient.BuildRequest(APIRouteSubUsers, rest.Get) | |||
| listReq.QueryParams = query | |||
| //listReq.QueryParams = query | |||
There was a problem hiding this comment.
I'll delete this
added 2 commits
February 19, 2020 11:35
remove commented code for query handling in sendgridapi.go that is no longer used.
aidenkeating
force-pushed
the
refactor-cli
branch
from
332648c to
e5afbea
Compare
ciaranRoche
approved these changes
Feb 19, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor the cli of the smtp-service to have a structure more
similar to the default cobra generated file structure.
along with this, other changes have been made:
verification:
'version', do the following steps
different string and ensure the output secret name is overridden
that debug and info logs are shown and they're output to stderr