Skip to content
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

Add emissary ingress CRD #123

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexbowers
Copy link

No description provided.

Copy link
Owner

@tommy351 tommy351 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please provide tests and a readme file.

@@ -0,0 +1,48 @@
{
"name": "@kubernetes-models/emissary-ingress",
"version": "3.6.0",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"version": "3.6.0",
"version": "0.0.0",

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, should this version not match the version that is provided in the CRDs? That was my understanding of it

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version doesn't reflect the CRD version.

third-party/emissary-ingress/package.json Outdated Show resolved Hide resolved
@jerolimov
Copy link
Contributor

@alexbowers are you still interested in this? If not, I could also take a look as part of a hacktoberfest contribution.

@alexbowers
Copy link
Author

alexbowers commented Oct 24, 2023 via email

@jerolimov
Copy link
Contributor

Sorry, I need more clarity. You are interested, but will you update the PR based on the feedback (version number, readme, tests), or should I try to open a new one?

@alexbowers
Copy link
Author

To be honest, I don't actually use Kubernetes Models TS anymore, i've instead moved to using CDK8s; so feel free to take this on, or close the PR and make your own

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants