Rename Files and Move Versioning Logic To Another Package #154
Conversation
aiyengar2
force-pushed
the
basic_refactor
branch
2 times, most recently
from
ed2cb52 to
18206d1
Compare
aiyengar2
changed the title
Signed-off-by: Arvind Iyengar <[email protected]> (cherry picked from commit 9ffb443)
Signed-off-by: Arvind Iyengar <[email protected]> (cherry picked from commit da06971)
aiyengar2
force-pushed
the
basic_refactor
branch
from
18206d1 to
83e4318
Compare
| @@ -1,4 +1,4 @@ | |||
| package helm | |||
| package chart | |||
There was a problem hiding this comment.
I'm curious about the rationale for this package move - the pattern for other projects that use Wrangler is to put the controller in pkg/foo/controller.go, for example system-upgrade-controller has its controller in package upgrade at pkg/upgrade/controller.go
There was a problem hiding this comment.
Is this in anticipation of having multiple controllers?
There was a problem hiding this comment.
Yes, the structure here follows what Fleet and Rancher do today. Example for Fleet: https://github.com/rancher/fleet/tree/master/pkg/controllers
There was a problem hiding this comment.
More importantly, this opens the door for moving the registration logic for controllers into a separate package instead of putting it in main.go and putting constants in a separate package.
i.e. the final structure looks like this: https://github.com/aiyengar2/helm-controller/tree/master/pkg/controllers
There was a problem hiding this comment.
Where the content that used to live in main.go is moved to the controllers package in https://github.com/aiyengar2/helm-controller/blob/e8055c48bf2cd3bc3bed5f311b501daf599ce28f/pkg/controllers/controllers.go#L37-L201
There was a problem hiding this comment.
thanks for the explanation. Sound like we might want to reorg system-upgrade-controller too at some point, if we're going to continue to maintain it.
| Version = "v0.0.0-dev" | ||
| GitCommit = "HEAD" |
| @@ -1,4 +1,4 @@ | |||
| package helm | |||
| package chart | |||
There was a problem hiding this comment.
thanks for the explanation. Sound like we might want to reorg system-upgrade-controller too at some point, if we're going to continue to maintain it.
Part 4 of #141