Refactor external add-ons section #3836
Conversation
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
google-oss-prow
bot
requested review from
adrian555,
ellistarn,
terrytangyuan and
woop
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
|
@andreyvelich @franciscojavierarceo @jbottum this is the PR to clean up the External Add-Ons section, I would appreciate your review. See the preview site here: |
| @@ -1,3 +0,0 @@ | |||
| approvers: | |||
| - woop | |||
| - franciscojavierarceo | |||
There was a problem hiding this comment.
Can we keep this file? Or rather make an equivalent feast one?
There was a problem hiding this comment.
The purpose of these sections is not to provide up to date information, it is to provide a brief "evergreen" explanation of what the add-on is, and how it relates to Kubeflow.
I would prefer if the root approvers of the Kubeflow website are aware and approve each proposed change, to avoid messaging conflicts.
Even without being an approver, you or others can raise PRs to propose updates.
Thank you for this! I left some small nits. Also, it may not really be necessary but I thought I'd mention that we've updated the architecture diagram on the feast documentation to reflect storing Request Sources which we plan to include in feast-dev/feast#4376
|
|
Changes look great to me, but my initial concern is still valid. There's missing instructions on how to deploy and integrate Feast with Kubeflow. @franciscojavierarceo any change to provide these instructions in Feast docs while we work to bring the manifests back in Kubeflow? |
I'm going to make a follow up PR with how to do this after I add the manifests back in but we shouldn't have to block on that imo. |
|
But @dmartinol @jeremyary we should discuss that |
|
I really like the structural changes @thesuperzapper ! Please just accommodate @franciscojavierarceo requests if you agree with them. /lgtm |
Signed-off-by: Mathew Wicks <5735406+thesuperzapper@users.noreply.github.com>
|
@franciscojavierarceo I have applied your updates, and added the diagram, please see the new preview site: |
|
@terrytangyuan we need a root approver because I had to fix files outside the KFP directory to avoid broken links. I'm pretty sure this is ready to merge now. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: franciscojavierarceo, terrytangyuan 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 |
This PR refactors and cleans up the "External Add-Ons" section.
The key changes this PR mages:
What is XXXX?and"How to use XXXX with Kubeflow?"In the "Introduction" pages, I have largley left the wording unchanged, and mostly tried to structure everything under the two core headings:
"What is XXXX?", and"How do I use XXXX with Kubeflow?"Screenshots
Section Overview
Feast Section