Skip to content
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 Katib API reference to the website docs #1531

Merged
merged 4 commits into from
Jan 16, 2020

Conversation

janeman98
Copy link
Contributor

@janeman98 janeman98 commented Jan 14, 2020

What this PR does / why we need it:
Add the Katib API reference to the website docs

Which issue(s) this PR fixes
Fixes #1481

Special notes for your reviewer:
@sarahmaddox
Please review. Please note that I use PyTorchJob as example rather than Metedata reference since the katib reference is the description of each field (like type, description, etc.) which is closer to that in PyTorchJob reference.

Question: there are two TODO in the original document in Github. How should we handle this in the website ? Leave them as TODO or remove them?


This change is Reviewable

@janeman98
Copy link
Contributor Author

/assign @sarahmaddox

@sarahmaddox
Copy link
Contributor

Copy link
Contributor

@sarahmaddox sarahmaddox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I've added two suggestions as review comments. One of them is a response to your question about the TODOs in the codel.

content/docs/reference/katib/v1alpha3/katib.md Outdated Show resolved Hide resolved
@sarahmaddox
Copy link
Contributor

@janeman98 My apologies! I see that now the intermediate _index.md file is gone, there's no way of seeing which version of the API the docs refer to, as the directory no longer appears in the breadcrumbs (see preview).

Please would you put the file back again? content/docs/reference/katib/v1alpha3/_index.md.

(@johnugeorge we should add the version to the API description. I've submitted a PR to suggest this change: kubeflow/katib#1017.)

@janeman98
Copy link
Contributor Author

@sarahmaddox No problem. I have added the file back : content/docs/reference/katib/v1alpha3/_index.md.

@sarahmaddox
Copy link
Contributor

Thanks @janeman98. Let's wait until my PR kubeflow/katib#1017 is reviewed. If the changes are accepted then we can just manually apply them to this PR (instead of regenerating).

@sarahmaddox
Copy link
Contributor

@janeman98 My PR was merged into the Katib repo. Would you like to apply the same updates manually to your PR? (I can do it if that's easier.) After that, this PR is ready to merge.

@janeman98
Copy link
Contributor Author

@sarahmaddox Thanks again.
I have applied the same updates manually to this PR. Please review/approve. Feel free to let me know if further modification is needed.

@johnugeorge
Copy link
Member

/lgtm

@sarahmaddox
Copy link
Contributor

Thanks @janeman98!
/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sarahmaddox

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 0134154 into kubeflow:master Jan 16, 2020
@janeman98 janeman98 deleted the add-katib-api branch January 16, 2020 20:42
@janeman98
Copy link
Contributor Author

Thanks @sarahmaddox !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hyperparameter tuning: Add the Katib API reference to the website docs
4 participants