Trainer: Documentation for Operator Guide
Thank you everyone!
Checklist:
- [Y] You have signed off your commits
- [Y] Ensure you follow best practices from our guide. Contributing.
- [N] You have included screenshots when changing the website style or adding a new page.
Description of your changes: Added Guides for Training Runtime and their Configurations of mlPolicy and Templete(Jobset)
Closes: # #2542
Labels
/area trainer
Hi @Garvit-77. Thanks for your PR.
I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andreyvelich @Electronic-Waste please review.
closes https://github.com/kubeflow/trainer/issues/2542
Hi @Garvit-77, did you get a chance to work on remaining items or we should re-assign this task ?
Hey @andreyvelich I wasn't much active recently ,but I will be pushing the changes by today EOD
@Garvit-77 Please let us know once you address the remaining comments, so we can take a look again.
Hey @andreyvelich , I have tried to resolve all the reviews by the latest commit ,please let me know if there are any further changes, I'm was a busy these days sorry for the delay thank you for the review and support!
@Garvit-77 Can you also rebase your PR and fix title so we can merge it ? /cc @varodrig @davidgs
@andreyvelich: GitHub didn't allow me to request PR reviews from the following users: davidgs.
Note that only kubeflow members and repo collaborators can review this PR, and authors cannot review their own PRs.
In response to this:
@Garvit-77 Can you also rebase your PR and fix title so we can merge it ? /cc @varodrig @davidgs
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Thank you for the review, I have made the all required changes @andreyvelich
@Garvit-77 please sign off your commits
Hey, @kramaranya Thank you for the review I noticed that most of the comments are around grammatical or synonymous phrasing. While I believe the technical content is clear, I understand the importance of consistency and will incorporate the suggestions where appropriate.
@Garvit-77 please sign off your commits Since the DCO check has passed, I assume that the commits are correctly signed off. Please let me know if there's anything further needed on this.
@Garvit-77 Please let us know once you get a chance to address @kramaranya suggestions, so we can move forward.
@andreyvelich comments were addressed by a commit earlier We can surely proceed with it.
@Garvit-77 I noticed that some of the suggestions you haven't addressed yet, for example:
- https://github.com/kubeflow/website/pull/4054/files#r2202449489
- https://github.com/kubeflow/website/pull/4054/files#r2202445366
@andreyvelich As the comments are around grammatical or synonymous phrasing, I incorporated some changes as the technical content is clearly defined.
As the comments are around grammatical or synonymous phrasing
I think, it would be nice if you could incorporate the grammatical feedback as well, so we don't forget to update it later. I agree with @kramaranya suggestions, since it makes docs easier to consume for our users.
Hey @andreyvelich, I have incorporated the changes.
Thank you for the update @Garvit-77! Did you get a chance to push changes for these comments two:
- https://github.com/kubeflow/website/pull/4054/files#r2202445366
- https://github.com/kubeflow/website/pull/4054/files#r2202442709
I think, after you've made these changes we can merge this PR.
I think, we can just move forward with this PR, I will address the comments in this PR: https://github.com/kubeflow/website/pull/4144
Thank you for this great contribution @Garvit-77! /lgtm /approve
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: andreyvelich
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~content/en/docs/components/trainer/OWNERS~~ [andreyvelich]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment