Conversation
|
should 'errConfigFileMissing' be totally removed? I see no references to it. |
cliconfig/config.go
Outdated
There was a problem hiding this comment.
weird golint doesn't highlight this one :/ probably because it's not underscored, I'd prefer to have it called ConfigFileName (or configFileName if it isn't exported and since configFile is declared below and would conflict otherwise) then, wdyt?
There was a problem hiding this comment.
I thought it strange as well. I'm guessing it's some odd case in golint.
This is the only exported usage I see.
registry/auth_test.go
37: root = filepath.Join(root, cliconfig.CONFIGFILE)
I agree on the recase.
There was a problem hiding this comment.
perfect, we can call it ConfigFileName so it's not all uppercase
There was a problem hiding this comment.
be awere CONFIGFILE is used even in config_file_test
There was a problem hiding this comment.
Of course, there is also a struct in this unit called ConfigFile. How about ConfigFilename?
There was a problem hiding this comment.
@MHBauer yes, either configFileName or configFilename will work
|
yep - couldn't find it - so kill it |
|
possibly a nit, should |
|
@MHBauer yes you could rename test file also |
- fully capitalize HTTP in HTTPHeaders - comment for CONFIGFILE - camelcase and privatize oldConfigfile, defaultIndexserver - remove unused var errConfigFileMissing - comments for methods and functions throughout - external references to renamed variables changed Signed-off-by: Morgan Bauer <mbauer@us.ibm.com>
|
LGTM assuming janky is happy |
|
LGTM |
There was a problem hiding this comment.
@MHBauer this is incorrect, it should be ConfigFileName. And please add cliconfig to the list in ./hack/make/validate-lint. You can send a new PR if you want :)
There was a problem hiding this comment.
Thanks for giving me the chance to fix this. see #15175
my part for #14756
Signed-off-by: Morgan Bauer mbauer@us.ibm.com