Skip to content

Commit

Permalink
Code review fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
@lkysow
lkysow committed Oct 29, 2017
1 parent fe773fe commit 3d361eb
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 26 deletions.
20 changes: 10 additions & 10 deletions cmd/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ import (
"github.com/spf13/viper"
)

// To add a new flag you must
// 1. Add a const with the flag name (in alphabetic order)
// 2. Add a new field to server.ServerConfig and set the mapstructure tag equal to the flag name
// 3. Add your flag's description etc. to the stringFlags, intFlags, or boolFlags slices
// To add a new flag you must:
// 1. Add a const with the flag name (in alphabetic order).
// 2. Add a new field to server.ServerConfig and set the mapstructure tag equal to the flag name.
// 3. Add your flag's description etc. to the stringFlags, intFlags, or boolFlags slices.
const (
AtlantisURLFlag = "atlantis-url"
ConfigFlag = "config"
Expand Down Expand Up @@ -150,13 +150,13 @@ Config file values are overridden by environment variables which in turn are ove
}),
}

// if a user passes in an invalid flag, tell them what the flag was
// If a user passes in an invalid flag, tell them what the flag was.
c.SetFlagErrorFunc(func(c *cobra.Command, err error) error {
fmt.Fprintf(os.Stderr, "\033[31mError: %s\033[39m\n\n", err.Error())
return err
})

// set string flags
// Set string flags.
for _, f := range stringFlags {
c.Flags().String(f.name, f.value, f.description)
if f.env != "" {
Expand All @@ -165,13 +165,13 @@ Config file values are overridden by environment variables which in turn are ove
s.Viper.BindPFlag(f.name, c.Flags().Lookup(f.name))
}

// set int flags
// Set int flags.
for _, f := range intFlags {
c.Flags().Int(f.name, f.value, f.description)
s.Viper.BindPFlag(f.name, c.Flags().Lookup(f.name))
}

// set bool flags
// Set bool flags.
for _, f := range boolFlags {
c.Flags().Bool(f.name, f.value, f.description)
s.Viper.BindPFlag(f.name, c.Flags().Lookup(f.name))
Expand All @@ -181,7 +181,7 @@ Config file values are overridden by environment variables which in turn are ove
}

func (s *ServerCmd) preRun() error {
// if passed a config file then try and load it
// If passed a config file then try and load it.
configFile := s.Viper.GetString(ConfigFlag)
if configFile != "" {
s.Viper.SetConfigFile(configFile)
Expand All @@ -208,7 +208,7 @@ func (s *ServerCmd) run() error {
}
sanitizeGithubUser(&config)

// config looks good, start the server
// Config looks good. Start the server.
server, err := s.ServerCreator.NewServer(config)
if err != nil {
return errors.Wrap(err, "initializing server")
Expand Down
32 changes: 16 additions & 16 deletions cmd/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (s *ServerStarterMock) Start() error {
}

func TestExecute_NoConfigFlag(t *testing.T) {
t.Log("If there is no config flag specified Execute should return nil")
t.Log("If there is no config flag specified Execute should return nil.")
c := setup(map[string]interface{}{
cmd.ConfigFlag: "",
cmd.GHUserFlag: "user",
Expand All @@ -43,7 +43,7 @@ func TestExecute_NoConfigFlag(t *testing.T) {
}

func TestExecute_ConfigFileExtension(t *testing.T) {
t.Log("If the config file doesn't have an extension then error")
t.Log("If the config file doesn't have an extension then error.")
c := setup(map[string]interface{}{
cmd.ConfigFlag: "does-not-exist",
cmd.GHUserFlag: "user",
Expand All @@ -54,7 +54,7 @@ func TestExecute_ConfigFileExtension(t *testing.T) {
}

func TestExecute_ConfigFileMissing(t *testing.T) {
t.Log("If the config file doesn't exist then error")
t.Log("If the config file doesn't exist then error.")
c := setup(map[string]interface{}{
cmd.ConfigFlag: "does-not-exist.yaml",
cmd.GHUserFlag: "user",
Expand All @@ -65,7 +65,7 @@ func TestExecute_ConfigFileMissing(t *testing.T) {
}

func TestExecute_ConfigFileExists(t *testing.T) {
t.Log("If the config file exists then there should be no error")
t.Log("If the config file exists then there should be no error.")
tmpFile := tempFile(t, "")
defer os.Remove(tmpFile)
c := setup(map[string]interface{}{
Expand All @@ -78,7 +78,7 @@ func TestExecute_ConfigFileExists(t *testing.T) {
}

func TestExecute_InvalidConfig(t *testing.T) {
t.Log("If the config file contains invalid yaml there should be an error")
t.Log("If the config file contains invalid yaml there should be an error.")
tmpFile := tempFile(t, "invalidyaml")
defer os.Remove(tmpFile)
c := setup(map[string]interface{}{
Expand All @@ -97,7 +97,7 @@ func TestExecute_Validation(t *testing.T) {
expErr string
}{
{
"should validate log level",
"Should validate log level.",
map[string]interface{}{
cmd.LogLevelFlag: "invalid",
cmd.GHUserFlag: "user",
Expand All @@ -106,14 +106,14 @@ func TestExecute_Validation(t *testing.T) {
"invalid log level: not one of debug, info, warn, error",
},
{
"should ensure github user is set",
"Should ensure github user is set.",
map[string]interface{}{
cmd.GHTokenFlag: "token",
},
"--gh-user must be set",
},
{
"should ensure github token is set",
"Should ensure github token is set.",
map[string]interface{}{
cmd.GHUserFlag: "user",
},
Expand All @@ -130,7 +130,7 @@ func TestExecute_Validation(t *testing.T) {
}

func TestExecute_Defaults(t *testing.T) {
t.Log("should set the defaults for all unspecified flags")
t.Log("Should set the defaults for all unspecified flags.")
c := setup(map[string]interface{}{
cmd.GHUserFlag: "user",
cmd.GHTokenFlag: "token",
Expand All @@ -157,7 +157,7 @@ func TestExecute_Defaults(t *testing.T) {
}

func TestExecute_ExpandHomeDir(t *testing.T) {
t.Log("should expand the ~ in the home dir to the actual home dir")
t.Log("Should expand the ~ in the home dir to the actual home dir.")
c := setup(map[string]interface{}{
cmd.GHUserFlag: "user",
cmd.GHTokenFlag: "token",
Expand All @@ -172,7 +172,7 @@ func TestExecute_ExpandHomeDir(t *testing.T) {
}

func TestExecute_GithubUser(t *testing.T) {
t.Log("should remove the @ from the github username if it's passed")
t.Log("Should remove the @ from the github username if it's passed.")
c := setup(map[string]interface{}{
cmd.GHUserFlag: "@user",
cmd.GHTokenFlag: "token",
Expand All @@ -184,7 +184,7 @@ func TestExecute_GithubUser(t *testing.T) {
}

func TestExecute_Flags(t *testing.T) {
t.Log("should use all flags that are set")
t.Log("Should use all flags that are set.")
c := setup(map[string]interface{}{
cmd.AtlantisURLFlag: "url",
cmd.DataDirFlag: "path",
Expand All @@ -211,7 +211,7 @@ func TestExecute_Flags(t *testing.T) {
}

func TestExecute_ConfigFile(t *testing.T) {
t.Log("Should use all the values from the config file")
t.Log("Should use all the values from the config file.")
tmpFile := tempFile(t, `---
atlantis-url: "url"
data-dir: "path"
Expand Down Expand Up @@ -241,7 +241,7 @@ require-approval: true`)
}

func TestExecute_EnvironmentOverride(t *testing.T) {
t.Log("Environment variables should override config file flags")
t.Log("Environment variables should override config file flags.")
tmpFile := tempFile(t, "gh-user: config\ngh-token: config2")
defer os.Remove(tmpFile)
os.Setenv("ATLANTIS_GH_TOKEN", "override")
Expand All @@ -254,7 +254,7 @@ func TestExecute_EnvironmentOverride(t *testing.T) {
}

func TestExecute_FlagConfigOverride(t *testing.T) {
t.Log("Flags should override config file flags")
t.Log("Flags should override config file flags.")
os.Setenv("ATLANTIS_GH_TOKEN", "env-var")
c := setup(map[string]interface{}{
cmd.GHUserFlag: "user",
Expand All @@ -266,7 +266,7 @@ func TestExecute_FlagConfigOverride(t *testing.T) {
}

func TestExecute_FlagEnvVarOverride(t *testing.T) {
t.Log("Flags should override environment variables")
t.Log("Flags should override environment variables.")
tmpFile := tempFile(t, "gh-user: config\ngh-token: config2")
defer os.Remove(tmpFile)
c := setup(map[string]interface{}{
Expand Down

0 comments on commit 3d361eb

Please sign in to comment.