Add support for a custom success error message. #72
Conversation
This allows users to personalize their response when no violations are found.
Codecov Report
@@ Coverage Diff @@
## main #72 +/- ##
==========================================
+ Coverage 95.48% 95.52% +0.03%
==========================================
Files 21 21
Lines 465 469 +4
==========================================
+ Hits 444 448 +4
Misses 12 12
Partials 9 9
Continue to review full report at Codecov.
|
cmd/root.go
Outdated
| @@ -112,7 +112,11 @@ func rootRunE(cmd *cobra.Command, args []string) error { | |||
| } | |||
|
|
|||
| if violations == 0 { | |||
| fmt.Fprintln(output.Stdout, "No violations found. Stay woke \u270a") | |||
| successExitMessage := "No violations found. Stay woke \u270a" | |||
There was a problem hiding this comment.
It might be worth utilizing viper's defaults for this https://github.com/spf13/viper#establishing-defaults. This would allow you to support the case where one may wish to set successExitMessage to an empty string. I believe initConfig would be a good place for it.
There was a problem hiding this comment.
Looks like config package loads directly from yaml.Unmarshal in loadConfig ignoring any viper configuration. where rootRunE calls cfg, err := config.NewConfig(viper.ConfigFileUsed()) which is only a path reference.
There was a problem hiding this comment.
Ah yes, that's right. viper doesn't fully manage the config file yet, I had to restrict it to yaml because I haven't had the chance to port the unmarshaling over yet.
| @@ -74,6 +77,20 @@ func TestRunE(t *testing.T) { | |||
| expected := "No violations found. Stay woke \u270a\n" | |||
| assert.Equal(t, expected, got) | |||
| }) | |||
|
|
|||
| t.Run("no violations found with custom message", func(t *testing.T) { | |||
There was a problem hiding this comment.
Would you consider a test case for setting the custom success message to an empty string?
There was a problem hiding this comment.
added this to config_test.go 👍
While the tool focuses on inclusivity (great!) it could also be used for additional language linting. Highlighting common grammatical mistakes, or used to correct common mispellings of internal service names or to promote an end to service name that is longer longer available. The other reason being while trying to adopt this tool, some feedback that the literal word |
refactor config so this can be set to be a literal empty string as well
|
@platinummonkey this change has been released in v0.8.0. Thanks again for the contribution! |
Please check if the PR fulfills these requirements
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
Currently the success message is statically set to
"No violations found. Stay woke \u270a"What is the new behavior (if this is a feature change)?
This allows users to personalize their response when no violations are found using the config file via the optional
success_exit_message.Does this PR introduce a breaking change? (What changes might users need to make due to this PR?)
Non-breaking change.
Other information: