pkg/daemon: idiomatic Go cleanup #369
Conversation
openshift-ci-robot
added
the
size/L
runcom
force-pushed
the
cleanup-idiomatic
branch
from
ce28159
to
b450219
Compare
runcom
force-pushed
the
cleanup-idiomatic
branch
2 times, most recently
from
65ce691
to
f5390aa
Compare
|
@runcom this is the best thank you! checking it out! |
runcom
force-pushed
the
cleanup-idiomatic
branch
4 times, most recently
from
26d0227
to
e6f8364
Compare
|
/test e2e-aws |
| @@ -12,7 +12,7 @@ import ( | |||
| "k8s.io/apimachinery/pkg/runtime" | |||
| kscheme "k8s.io/client-go/kubernetes/scheme" | |||
|
|
|||
| "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" | |||
| v1 "github.com/openshift/machine-config-operator/pkg/apis/machineconfiguration.openshift.io/v1" | |||
pkg/daemon/update.go
Outdated
| if err := dn.writeUnits(newConfig.Spec.Config.Systemd.Units); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := dn.deleteStaleData(oldConfig, newConfig); err != nil { |
There was a problem hiding this comment.
nit: if in future you add another step after deleteStaleData, you'll have to bring back this if... The current setup keeps it clearer imo.
There was a problem hiding this comment.
Boils down to preference for me, if you prefer it that way I'll change it back
pkg/daemon/writer.go
Outdated
| @@ -4,7 +4,7 @@ import ( | |||
| "fmt" | |||
|
|
|||
| "github.com/golang/glog" | |||
| "k8s.io/api/core/v1" | |||
| v1 "k8s.io/api/core/v1" | |||
There was a problem hiding this comment.
Why this? Maybe corev1?
There was a problem hiding this comment.
This is golint doing this, overlooked
Went through a bunch of cleanup to be more idiomatic when writing Go code (no else if previous return, default values, return on err, bool comparisons etc etc) (mainly went to read and learn the code) There should be no functional change at all. Signed-off-by: Antonio Murdaca <runcom@linux.com>
runcom
force-pushed
the
cleanup-idiomatic
branch
from
e6f8364
to
181d09d
Compare
|
/approve |
openshift-ci-robot
added
the
approved
|
/lgtm |
|
Very nice! |
| degraded_err := nw.SetUpdateDegraded(err, client, node) | ||
| if degraded_err != nil { | ||
| glog.Errorf("Error while setting degraded: %v", degraded_err) | ||
| degradedErr := nw.SetUpdateDegraded(err, client, node) |
There was a problem hiding this comment.
To keep it consistent we should probably just make it if degradedErr := nw.SetUpdateDegraded(err, client, node); degradedErr != nil here right?
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, jlebon, runcom, yuqi-zhang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Went through a bunch of cleanup to be more idiomatic when writing Go
code (no else if previous return, default values, return on err, bool comparison etc etc)
(mainly went to read and learn the code)
There should be no functional change at all.
(so sorry for this PR 馃懠 )
Signed-off-by: Antonio Murdaca runcom@linux.com