-
Notifications
You must be signed in to change notification settings - Fork 795
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
refactor "Installing Kubeflow" page #2611
refactor "Installing Kubeflow" page #2611
Conversation
@RFMVasconcelos @cvenets |
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 few suggestions
@joeliedtke I think maybe you want to take a look at my PR instead. Most of what you're commenting on, I've already fixed on here: #2625 |
@RFMVasconcelos @joeliedtke I have made changes based off the comments, any further thoughts? After we agree on the bullet points under each section, we can write the actual text. |
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.
@thesuperzapper thank you for leading this! I have left a few comments throughout :)
@thesuperzapper Thank you for your last commits, this is shaping up to be a great overview! 🎉 Please see this comment. Aside from that, I think we are ready to write the missing descriptions and effectively merge this. WDYT? |
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.
@shannonbradshaw I think we should update this page centrally here. I have adapted pieces from your #2625 PR to complete this one so we can maximize output and take advantage of everyone's willingness to contribute.
@thesuperzapper can you review the text proposals and, with your edits, commit so we can get to a final PR?
@RFMVasconcelos sounds good! Thanks for all the work, @thesuperzapper and @RFMVasconcelos. |
@RFMVasconcelos I have incorporated your suggestions, and I think this is ready to Obviously we will iterate on this page, but we should try and get this merged very soon, so it can be linked to in the 1.3 blogs. |
@thesuperzapper Arrikto Enterprise Kubeflow should be listed among the distributions. Asking that we include at: with the text below `+++ The Arrikto Enterprise Kubeflow (EKF) distribution extends the capabilities of the Kubeflow platform with additional automation, reproducibility, portability, and security features.
|
@thesuperzapper the column values for the table entry for the EKF distribution should be: |
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.
@thesuperzapper, requesting a few changes.
</tr> | ||
<tr> | ||
<td>Kubeflow on Openshift</td> | ||
<td>IBM Cloud</td> |
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.
Red Hat
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.
Currently, this distribution is maintained by IBM Cloud, (not RedHat) @animeshsingh can verify.
<td>Cloud Marketplaces, Vagrant</td> | ||
<td><a href="/docs/distributions/minikf/">Docs</a></td> | ||
<td></td> | ||
</tr> |
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.
<tr> <td><a href="/docs/distributions/ekf/">Arrikto Enterprise Kubeflow</a></td> <td>Arrikto</td> <td>EKS, AKS, GKE </td> </tr>
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.
Lets keep this PR to refactoring what was already there, if you want to add another doc page, you can just raise a PR which updates the "Installing Kubeflow" page, after this has been merged.
@shannonbradshaw you mentioned wanting to add the "Arrikto Enterprise Kubeflow" distribution, please raise a separate PR (after this one is merged), as this PR is only for refactoring the "Installing Kubeflow" page, not adding a new distribution doc. |
@RFMVasconcelos I think this is good enough to merge now so we have a proper "Installing Kubeflow" page up by the time 1.3 is dropped tonight. We can keep iterating on it over the next while if needed. |
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.
@thesuperzapper @shannonbradshaw for the interest of time I agree with @thesuperzapper that we should merge this and then keep on iterating in further PRs.
For reference, this will merge to the master
branch. For these changes to be effective we need @Bobgy or @joeliedtke to change the config in netlify so that the website points to that branch.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RFMVasconcelos, thesuperzapper 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 |
Adding some late comments:
EDIT: 1 and 2 already fixed by #2635 |
I really love where this is going and appreciate the efforts! |
Sorry if I prematurely merged this one @Bobgy, but wanted to accelerate the progress, allowing iteration in further PRs. :) Blockers:
|
Closes #2590
As discussed in Issue #2590, this is a rewrite for the "Installing Kubeflow" page.