Refactor and add tests, implement a feature #7
Conversation
|
I've typically seen the |
|
Lgtm fwiw. |
| return clnt, err | ||
| } | ||
|
|
||
| func (p *policyEvaluator) fetchEnterpriseContractPolicy(ctx context.Context) (*ecp.EnterpriseContractPolicy, error) { |
There was a problem hiding this comment.
I see this is a function defined for a struct in another file. I understand the relation between this function and the kubernets_client.go file, but does it hurt readability by having functions separated like this?
There was a problem hiding this comment.
Missed the refactoring there, I wanted a separate struct for k8s, that should not belong on policyEvaluator
There was a problem hiding this comment.
I've left a few notes.
A few code style issues that should be addressed:
- go uses camel case. no snake casing.
- all functions should have appropriate doc strings
- ensure a blank line between license boilerplate and package declaration to avoid tools such as
golintfrom complaining.
https://github.com/golang-standards/project-layout#internal
|
What we didn't have in the `bash` or the `ec-cli` was the ability to have multiple sources of policy (Rego) files, e.g. from multiple git repositories or branches. Now this is available in the `ec-cli`. This does however refactor the implementation into smaller more manageable files, hopefully makes it easier to maintain. Tests were added. Ref. https://issues.redhat.com/browse/HACBS-372
What we didn't have in the
bashor theec-cliwas the ability tohave multiple sources of policy (Rego) files, e.g. from multiple git
repositories or branches. Now this is available in the
ec-cli.This does however refactor the implementation into smaller more
manageable files, hopefully makes it easier to maintain.
Tests were added.
Ref. https://issues.redhat.com/browse/HACBS-372