-
Notifications
You must be signed in to change notification settings - Fork 135
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
The kfp setup.md file needs to tell you to run make setup command in the kind directory. #252
Conversation
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
kfp/doc/setup.md
Outdated
@@ -66,10 +66,11 @@ choose your OS system, and process according to "(Optional) Install the MinIO Cl | |||
|
|||
## Installation steps <a name = "installation"></a> | |||
|
|||
You can create a Kind cluster with all required software installed using the following command: | |||
You can create a Kind cluster with all required software installed using the following command, run in the `kind` directory under the repo root: |
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.
Hi @deanwampler , sorry maybe it is my English, but the sentence is not clear to me especially that the first command is cd kind
.
Will it be better:
You can create a Kind cluster with all required software installed using the following command:
make -C kind setup
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.
Maybe I missed that. Sorry.
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.
Pushing an update...
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
Signed-off-by: Dean Wampler <[email protected]>
I think the only "meaningful" difference left is the suggestion for an |
@@ -27,6 +28,8 @@ include .make.defaults | |||
# Global rules that are generally to be implemented in the sub-directories and can | |||
# be overridden there (the double colon on the rule makes the overridable). | |||
|
|||
all: clean test build |
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 don't know if we're ready for this, in particular build
is ill-defined.
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.
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.
The typo is fixed by #333.
The kind directory is going to be refactored in a separate PR, and the README file in the root directory will be updated accordingly.
@deanwampler , thank you for your review and the PR. We will update the instructions on how to install a Kind cluster. |
Note: This PR is based off my previous
deanw-pr
. The only new change compared to that PR is to one file:kfp/doc/setup.md
.Why are these changes needed?
Without telling you to run the
make setup
command in thekind
directory, you'll guess you should do it in thekfp
directory, but the target isn't defined there.Related issue number (if any).
N/A