-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
use common stackManager interface and fakes across packages #3216
use common stackManager interface and fakes across packages #3216
Conversation
108f80b
to
aa6ae5e
Compare
aa6ae5e
to
46b8492
Compare
clientSet kubeclient.Interface | ||
} | ||
|
||
//go:generate counterfeiter -o fakes/fake_stack_manager.go . StackManager | ||
type StackManager interface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These custom interfaces are actually kind of useful for seeing which parts of the StackManager
are used around the codebase, could be a guide to splitting it up
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I don't disagree, me and @Callisto13 had a chat about wether to keep these or not in the issue #2931 (comment), I'm quite torn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mike does have a point that it could make things easier when we come to splitting that beast of a stack manager.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does, but like you pointed out in the issue it means loads of duplication in what is likely every package and if I had to guess would make people writing tests less likely 🤷 I'm leaning towards this at the moment as it enables everyone to very quick start adding tests
46b8492
to
213cb46
Compare
213cb46
to
2b0a176
Compare
2b0a176
to
ccdd3ab
Compare
Description
First of many small PRs relating to #2931. This introduces a stackManager interface and fake that can be used to mock out the package directly, it should make testing this in the future easier.
This also highlights how INSANELY big the stack collection is, we should try to refactor this out into smaller sub packages in the future
Checklist
README.md
, or theuserdocs
directory)area/nodegroup
) and kind (e.g.kind/improvement
)BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯