-
Notifications
You must be signed in to change notification settings - Fork 554
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
[EN] add ingress #2397
[EN] add ingress #2397
Conversation
Signed-off-by: aakankshabhende <[email protected]>
✅ Deploy Preview for cncfglossary ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
@aakankshabhende thank you so much for your efforts and patience. We are almost there. I made some slight suggestions, please take a look. Good job!
Signed-off-by: aakankshabhende <[email protected]>
05f051c
to
599e84c
Compare
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.
lgtm.
- tech check passed
To see where this is in the review pipeline and follow the progress, please look at the definition review board. |
@aakankshabhende I realized that the spellcheck is failing due to a typo. PTAL :) |
Yes will send a fix |
@nate-double-u Please check this PR, I have fixed the spellcheck. |
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.
I will paraphrase one of my colleagues, @seokho-son, that this definition is scoped only for Kubernetes Ingress (and its controller). I suggest reworking this into a more general cloud native-wide definition since ingress is a general concept in cloud native.
Also, in the context of general ingress, I'm curious if we should capitalize "Ingress." I think we should when we are refering to the Kubernetes Ingress resource specifically, but as I mention above, I think we should look to a more general definition of ingress here.
Also, and sorry I missed this on my first post, thanks for your effort on this @aakankshabhende, it is appreciated 🙂 To add to my earlier comments, I think it's OK to mention Kubernetes, I'm just concerned that as is, this reads like Ingress is only a K8s thing. |
Signed-off-by: aakankshabhende <[email protected]>
2cef796
to
448e3f1
Compare
@nate-double-u Please review my changes |
@nate-double-u Could you please review my changes? |
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.
Thanks for the updates @aakankshabhende! And sorry for my delay reviewing. I’ve made some comments inline.
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.
Just a few more comments from me.
To see where this is in the review pipeline and follow the progress, please look at the definition review board. |
Signed-off-by: aakankshabhende <[email protected]>
@hlipsig I made the suggested changes. Please review. |
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.
Thanks for the updates @aakankshabhende, this is a good article. And, thanks to all the reviewers who helped out!
To see where this is in the review pipeline and follow the progress, please look at the definition review board. |
Describe your changes
Added ingress definition to glossary in english.
Related issue number or link (ex:
resolves #issue-number
)Resolves #422
Checklist before opening this PR (put
x
in the checkboxes)git commit -s
) is to affirm that commits comply DCO. If you are working locally, you could add an alias to yourgitconfig
by runninggit config --global alias.ci "commit -s"
.