pkg/controller/template: get rid of pointer to map #380
Conversation
openshift-ci-robot
requested review from
kikisdeliveryservice and
smarterclayton
openshift-ci-robot
added
approved
Just do this as basic gotchas in Go even if it was a nice hack. Signed-off-by: Antonio Murdaca <runcom@linux.com>
runcom
force-pushed
the
no-pointer-map
branch
2 times, most recently
from
f7d46f7
to
fa1f8bf
Compare
Signed-off-by: Antonio Murdaca <runcom@linux.com>
runcom
force-pushed
the
no-pointer-map
branch
from
fa1f8bf
to
80e990a
Compare
- add comments to exported symbols - fix golint Signed-off-by: Antonio Murdaca <runcom@linux.com>
openshift-ci-robot
added
size/L
|
This looks good to me! Will let others have a look. |
|
I'm generally OK with this but one thing we need to keep in mind with general refactoring is that today we don't really have many tests for the MCO in general - see #257. Code LGTM just saying let's be careful 😄 |
|
Yeah, I fear regressions at any time here but this code path has unit tests which I've tested they can catch up failures. I generally agree though |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, jlebon, runcom 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 |
Just do this as basic gotchas in Go even if it was a nice hack (maps are pointers anyway so just call a func for readability mainly). Unit tests cover this code path.
The second commit of this PR is just moving floating strings to consts (even if we'll grab those from elsewhere in the future, we avoid having strings around)
The third commit is a usual cleanup
Signed-off-by: Antonio Murdaca runcom@linux.com