-
Notifications
You must be signed in to change notification settings - Fork 55
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
Slight cleanup of some of our readmes #221
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kfswain The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Deploy Preview for gateway-api-inference-extension ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Is this part of #202? |
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.
To provide clear separation between the Gateway implementation and the extension, est_proc.yaml
should be in a separate directory, e.g. ./pkg/manifests/epp
, ./pkg/manifests/inference-extension
, etc. (xref)
If so, please add Fixes: <Issue #> |
pkg/README.md
Outdated
1. **Install the CRDs into the cluster:** | ||
|
||
```sh | ||
make install |
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.
Considering using kubectl apply -f config/crd/bases
for consistency and some no-dev users are more likely to have kubectl installed than make.
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.
+1, this will point to a released manifest once we release
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.
Added a second option for the user to pick
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.
Actually, I am wondering if we can have the guide to not require cloning the repo? This significantly reduces the barrier to testing it out.
All the kubectl commands can be run against the files from github directly.
…at depends on envoy gateway
pkg/README.md
Outdated
1. **Install the CRDs into the cluster:** | ||
|
||
```sh | ||
make install |
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.
+1, this will point to a released manifest once we release
once this merges, we will have the manifests in the right places and we can update the kubectl commands to point to github directly so that we don't require cloning the repo to run the guide. /lgtm |
Part of #202