Minor/boring change: Consistently return user facing errors in SDN #13154
Conversation
|
@openshift/networking ptal |
|
Is the lower-case convention true for acronyms too? |
|
Hm, not sure. @smarterclayton ^^ |
|
It would not be true for acronyms in the beginning of error messages - they can stay as is. |
|
From https://github.com/golang/go/wiki/CodeReviewComments#error-strings:
(So the glog/echo changes should also be reverted.) |
pravisankar
force-pushed
the
fix-sdn-lowercase
branch
from
232c1e0 to
e310485
Compare
|
Updated, ptal |
|
[test] |
pkg/sdn/plugin/master.go
Outdated
| @@ -120,32 +120,32 @@ func (master *OsdnMaster) validateNetworkConfig() error { | |||
| // Ensure cluster and service network don't overlap with host networks | |||
| for _, ipNet := range hostIPNets { | |||
| if ipNet.Contains(ni.ClusterNetwork.IP) { | |||
| errList = append(errList, fmt.Errorf("Error: Cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String())) | |||
| errList = append(errList, fmt.Errorf("error: Cluster IP: %s conflicts with host network: %s", ni.ClusterNetwork.IP.String(), ipNet.String())) | |||
There was a problem hiding this comment.
Can you drop the "error:" prefix here? (4 cases in this loop, plus another 2 later on in this file)
Other than that LGTM
Follow the existing convention: errors should always start with lowercase https://github.com/golang/go/wiki/CodeReviewComments#error-strings
pravisankar
force-pushed
the
fix-sdn-lowercase
branch
from
e310485 to
c102653
Compare
|
[merge] |
1 similar comment
|
[merge] |
|
Evaluated for origin test up to c102653 |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/762/) (Image: devenv-rhel7_6027) |
|
Evaluated for origin merge up to c102653 |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/762/) (Base Commit: b6847b3) |
Follow the existing convention: user facing errors should always start with lowercase.