flow control: refactor and group dependencies in FlowController constructor#2775
Conversation
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Hi @evacchi. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/lgtm
Since we control all callers here, this is fine. I didn't want unnecessary defensive checks. Though, this is usually more of a concern for anything running on the hot path. |
|
/ok-to-test |
|
/retest (possibly a flake) |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahg-g, evacchi The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
…ructor (kubernetes-sigs/gateway-api-inference-extension#2775) * wip Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com> * wip2 Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com> --------- Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Follow-up to @LukeAVanDrie's suggestion in #2530 (comment)
Refactor
controller.NewFlowController()to accept acontroller.Deps{}struct with the expected parameters. This also makes theoptsvarargs probably less useful, so this PR proposes to remove that too.Notice error checking is pretty minimal, i.e. we only default the clock to a RealClock when the field is nil, otherwise we assume the rest of the parameters are correct. I can add more error checking if it's more consistent with the rest of the codebase.
Which issue(s) this PR fixes:
Fixes #2764
Does this PR introduce a user-facing change?: