Move ResourceID from root to resource package #2201
Conversation
stefanprodan
commented
Jun 27, 2019
•
stefanprodan
commented
- move ResourceID from root to resource package
- move Update type from policy pkg to resource pkg to avoid circular dependency
- rename resource.Update to resource.PolicyUpdate
- rename ResourceID and related functions to ID
- run go fmt on all files
|
ResourceID should really be |
stefanprodan
changed the title
There was a problem hiding this comment.
-
the resource import seems to have got jammed among stdlib imports, wherever it's mentioned. It should be among the weaveworks/flux imports.
-
resource.Update shouldn't really be in that package (see comment in resource/policy.go)
Elsewise, a few accidental replacements, for which I've put in suggestions.
| "github.com/weaveworks/flux/api/v10" | ||
| "github.com/weaveworks/flux/api/v6" | ||
| ) | ||
|
|
||
| type ListServicesOptions struct { | ||
| Namespace string | ||
| Services []flux.ResourceID | ||
| Services []resource.ID |
There was a problem hiding this comment.
This is SO much nicer, it is like a breath of fresh air!
| @@ -145,7 +145,7 @@ func (c *Cluster) ImagesToFetch() registry.ImageCreds { | |||
|
|
|||
| imageCreds := make(registry.ImageCreds) | |||
| for _, workload := range workloads { | |||
| logger := log.With(c.logger, "resource", flux.MakeResourceID(ns.Name, kind, workload.GetName())) | |||
| logger := log.With(c.logger, "resource", resource.MakeID(ns.Name, kind, workload.GetName())) | |||
| "github.com/weaveworks/flux/cluster" | ||
| "github.com/weaveworks/flux/cluster/kubernetes/resource" | ||
| kresource "github.com/weaveworks/flux/cluster/kubernetes/resource" |
There was a problem hiding this comment.
Good to have consistency here, nice
resource/policy.go
Outdated
|
|
||
| type Updates map[ID]Update | ||
|
|
||
| type Update struct { |
There was a problem hiding this comment.
This is a weird place to put this type. At the least, I'd expect it to be called PolicyUpdate. But it probably belongs in another package, rather than here. As update.Policy, if that doesn't create more problems, for example.
|
@squaremo I've went with renaming |
Was it impossible to do the preferred change, moving it into |
|
Ugh, nightmare. OK. |
|
👍 Rebass and squash the suggestion commit, and the Update re-rename. |
- rename ResourceID and related functions to ID - move Update type from policy pkg to resource pkg to avoid circular dependency - rename resource.Update to resource.PolicyUpdate - run go fmt on all files
Move ResourceID from root to resource package
