Conversation
Stevenjin8
requested review from
LiorLieberman,
robscott and
youngnick
as code owners
k8s-ci-robot
added
the
approved
k8s-ci-robot
added
size/L
|
|
||
| // Emit converts the provider intermediate representation to Gateway API resources. | ||
| func (e *Emitter) Emit(ir emitterir.EmitterIR) (i2gw.GatewayResources, field.ErrorList) { | ||
| utils.LogUnparsedErrors(ir, notify) |
There was a problem hiding this comment.
what do you think about moving this function to ingress2gateway.go around L104?
That way, none of the emitters need to be aware of this function, which helps prevent missing output from some emitters.
But, there’s also the consideration that structurally we’d like to handle emitterIR within the emitter package.
There was a problem hiding this comment.
This is what I originally had in mind. I ran into two problems
- Emitters output
GatewayResourceswhich do not have IR. So for this to work, Emitters would need to output bothGatewayResourcesand emitter IR, which seems a bit awkward - I like the idea that emitters have control over what they output.
k8s-ci-robot
added
the
needs-rebase
|
LGTM, thanks! |
Stevenjin8
force-pushed
the
feature/unparsed-ir
branch
from
37459ad to
48c0ce8
Compare
k8s-ci-robot
removed
the
needs-rebase
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kkk777-7, Stevenjin8 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 |
|
LGTM, thanks! |
k8s-ci-robot
added
the
lgtm
3 participants
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?:
cc @kkk777-7