feat: kgateway emitter#320
Conversation
|
Hi @puertomontt. 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 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. |
|
/ok-to-test |
| return filepath.Dir(goModPath) | ||
| } | ||
|
|
||
| func canonicalizeMultiDocYAML(t *testing.T, in []byte) []byte { |
There was a problem hiding this comment.
We've got integration tests in work, so we can probably do those tests here.
|
@puertomontt could you add yourself to the owners file and agree to ingress2gateway/docs/emitters.md Lines 61 to 83 in 2802595 |
|
@puertomontt Also, it would be nice to add some e2e tests, but we can have that in another PR (sooner rather than later hopefully) |
|
@Stevenjin8 would it be alright to merge this first then add myself as an owner. I need one merged PR to request org membership or if you sponsor me, can I be added to the org? |
|
@Stevenjin8 I'll apply for org after this merges. |
|
@puertomontt happy to sponsor. |
|
@Stevenjin8 can we merge this while waiting for me to be added to k8s sigs org? we have a lot of follow up PR we want to start on. Thanks |
a39deda to
3268c59
Compare
|
Fix: #349 |
|
@johananl @Stevenjin8 is this good to go now? |
|
@puetromontt PRs need two reviews (and approve and an lgtm) to merge. I already approved it. (But PRs from maintainer only need one) |
I haven't reviewed your code yet because Open-source projects rely very often on async communication. Please be patient 🙂 |
|
@puertomontt could you please rebase your branch on current |
johananl
left a comment
There was a problem hiding this comment.
Thank you for your contribution @puertomontt!
I left a few small comments/questions, the main one being about potentially unused code.
| t.Run("basic conversion", func(t *testing.T) { | ||
| runTestCase(t, &testCase{ | ||
| gatewayImplementation: kgatewayName, | ||
| providers: []string{ingressnginx.Name}, |
There was a problem hiding this comment.
Yes, I'm lookin into it. I assume you mean in general rather than as feedback on this specific PR @Stevenjin8.
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
Signed-off-by: omar <omar.hammami@solo.io>
7bb3fea to
43d3bdd
Compare
Signed-off-by: omar <omar.hammami@solo.io>
43d3bdd to
18a1147
Compare
|
Thanks for following up. I think we're good. We do have a lot of inconsistencies on doc comment style but I'm wondering if we shouldn't just use a linter to enforce this (I know many people don't care about this too much). |
|
/lgtm |
|
Thank you for your contribution @puertomontt. |
|
Thanks for the reivew @johananl @Stevenjin8 |
* kgateway emitter Signed-off-by: omar <omar.hammami@solo.io> * remove test and fix year Signed-off-by: omar <omar.hammami@solo.io> * add puertomontt as owner Signed-off-by: omar <omar.hammami@solo.io> * remove from owners Signed-off-by: omar <omar.hammami@solo.io> * e2e test Signed-off-by: omar <omar.hammami@solo.io> * address feedback Signed-off-by: omar <omar.hammami@solo.io> * add emitter to testCase Signed-off-by: omar <omar.hammami@solo.io> * update test Signed-off-by: omar <omar.hammami@solo.io> * remove periods Signed-off-by: omar <omar.hammami@solo.io> * remove unused code Signed-off-by: omar <omar.hammami@solo.io> * go mod changes Signed-off-by: omar <omar.hammami@solo.io> --------- Signed-off-by: omar <omar.hammami@solo.io>
* kgateway emitter Signed-off-by: omar <omar.hammami@solo.io> * remove test and fix year Signed-off-by: omar <omar.hammami@solo.io> * add puertomontt as owner Signed-off-by: omar <omar.hammami@solo.io> * remove from owners Signed-off-by: omar <omar.hammami@solo.io> * e2e test Signed-off-by: omar <omar.hammami@solo.io> * address feedback Signed-off-by: omar <omar.hammami@solo.io> * add emitter to testCase Signed-off-by: omar <omar.hammami@solo.io> * update test Signed-off-by: omar <omar.hammami@solo.io> * remove periods Signed-off-by: omar <omar.hammami@solo.io> * remove unused code Signed-off-by: omar <omar.hammami@solo.io> * go mod changes Signed-off-by: omar <omar.hammami@solo.io> --------- Signed-off-by: omar <omar.hammami@solo.io>
What type of PR is this?
/kind feature
depends on #305
What this PR does / why we need it:
Add kgateway emitter.
Which issue(s) this PR fixes:
Fixes #260
Does this PR introduce a user-facing change?: