-
Notifications
You must be signed in to change notification settings - Fork 30
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
Re-arranging the documentation #148
Conversation
- Added how to create a repo - Secret for git repo
✅ Deploy Preview for nephio ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@vjayaramrh and @dkosteck please check if you are fine with my edit. Instead of using the term Real K8s I have changed it to Other K8s Distributions because the term real is misleading. Even other Other K8s Distributions is not the best choice but it's better than real. |
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.
/approve
Co-authored-by: Gergely Csatari <[email protected]>
Co-authored-by: Gergely Csatari <[email protected]>
@@ -93,7 +96,7 @@ sudo NEPHIO_DEBUG=false \ | |||
bash | |||
``` | |||
|
|||
**Real K8s Cluster** | |||
**Other K8s Distributions** |
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.
How about Pre-installed K8S clusters instead of Other K8s Distributions?
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 tend to agree, "Other" may also be ambiguous, so perhaps something along the lines of what @vjayaramrh suggested
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.
Okay sounds good, I will change it
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.
Hello @arora-sagar, I have added some suggestions that I think help with readability, some spelling/capitalization, etc. Feel free to disagree with them if you think it reads fine now. I also commented on Vish's response to "Real" clusters, and a clarifying question
@@ -93,7 +96,7 @@ sudo NEPHIO_DEBUG=false \ | |||
bash | |||
``` | |||
|
|||
**Real K8s Cluster** | |||
**Other K8s Distributions** |
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 tend to agree, "Other" may also be ambiguous, so perhaps something along the lines of what @vjayaramrh suggested
|
||
If you want to use Github or Gitlab then follow below steps | ||
|
||
Get a [GitHub token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens#fine-grained-personal-access-tokens) if your repository is private, or allow Porch to make modifications. |
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'm confused by the "or allow Porch to make modifications" portion of this step. Perhaps it's me being newer and not understanding, but should there be some clarification here? How would that be done?
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 left it how it was earlier, I suppose it should be "to allow Porch to make modifications"
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.
Right, I think that probably makes more sense in this context
- Changing from other k8s distributions to pre-installed
@dkosteck I have resolved the conversation for the changes you have suggested. Let me know if there is something left. |
Looks good. Thanks! |
/approve |
@CsatariGergely Can we merge it now? |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: CsatariGergely, liamfallon 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 |
Fixes: nephio-project/nephio#592 |
/lgtm |
```bash | ||
export NEPHIO_USER=$USER | ||
export ANSIBLE_CMD_EXTRA_VAR_LIST="k8s.context=[email protected] kind.enabled=false host_min_vcpu=4 host_min_cpu_ram=8" | ||
export ANSIBLE_CMD_EXTRA_VAR_LIST="k8s.context=kind-kind kind.enabled=false host_min_vcpu=4 host_min_cpu_ram=8" |
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.
@arora-sagar Is this captured in a the user provided environment varaibles table?
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.
@vjayaramrh This was already there I did add it, though I tried to run the script and frankly I failed so I am not sure if it is correct.
Tested single VM and multiple VM steps.
This PR is addressing nephio-project/nephio#592