-
Notifications
You must be signed in to change notification settings - Fork 781
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
Added outdated warning to all pipelines v1 pages #3775
Conversation
Skipping CI for Draft Pull Request. |
@hbelmiro looks good, simple fix. /lgtm |
@hbelmiro Although we may want to have a new "out of date" warning for all the pages in the "Legacy V1" section: https://www.kubeflow.org/docs/components/pipelines/legacy-v1/ Which explains that users are looking at the docs for Kubeflow Pipelines 1.0, which is no longer supported (but which Kubeflow Pipelines 2.0 supports as backwards compatible). |
@thesuperzapper something like the following?
cc @rimolive |
@hbelmiro I think something more direct about telling users to see the new docs (and linking them).
On the page which explains the backwards compatibility we should outline:
|
Signed-off-by: hbelmiro <[email protected]>
Signed-off-by: hbelmiro <[email protected]>
acf048d
to
1f34970
Compare
Added.
This is out of the scope of this PR. Can I ask you to open an issue? We can then add it to #3712. |
/lgtm |
@hbelmiro we can probably include most of the information people need to know in the warning itself, and just link them to the migration page. How about we use this warning:
Either way, I definitely think we can improve the V2 migration guide, because it's quite hard to follow right now, and does not really "sell you" on why you should go through the effort. |
@hbelmiro Open a follow-up issue with @thesuperzapper requests and let's move to other issues. There's much more important sections to cover in the new KFP docs. |
@rimolive this PR does not block anything, so there is no need to rush it? Also, I think it's very important for users to understand V1 vs V2 as right now its very confusing to users. It a very straightforward update I have suggested in #3775 (comment), and if @hbelmiro agrees, they can choose to update this PR so we can merge it. |
@thesuperzapper It does block the only contributor working on the remaining items for KFP docs, as listed in #3712. Your suggestion can be worked later when we have the other pending topics done. |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rimolive 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 |
@hbelmiro I am disappointed this PR was merged without responding to my comments in my review #3775 (comment). The update I proposed was very important, so I have raised a new PR update the text: |
Thanks for opening the issue. We'll be working to fix until 1.9 release. |
@thesuperzapper the issue that this PR addresses was originally to remove the warnings. You asked that here, I created the issue, and sent this PR. @rimolive left clear here that we needed to move on with this PR to fix other pending issues. |
Resolves: #3751