-
Notifications
You must be signed in to change notification settings - Fork 31
Added devfile registry docs #73
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 devfile registry docs #73
Conversation
johnmcollier
left a comment
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.
Generally looks fine to me. Just left a couple small comments
Co-authored-by: Rolfe Dlugy-Hegwer <rolfedh@users.noreply.github.com>
Co-authored-by: Rolfe Dlugy-Hegwer <rolfedh@users.noreply.github.com>
Co-authored-by: Rolfe Dlugy-Hegwer <rolfedh@users.noreply.github.com>
|
@johnmcollier if you could take another look and give it the LGTM if you think it looks good. Apologies for the many commits...I accepted Rolfe's changes than made several myself, resulting in lots of commits! Thanks for the help! |
Co-authored-by: Rolfe Dlugy-Hegwer <rolfedh@users.noreply.github.com>
rolfedh
left a comment
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.
One minor comment. Otherwise, /lgtm
|
@rolfedh: changing LGTM is restricted to collaborators DetailsIn response to this:
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. |
|
@rkratky gotten the LTM from docs and devs. Please let me know if there's anything you would like me to address or change before you merge, thanks! |
rkratky
left a comment
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.
@jc-berger, comments added.
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
Co-authored-by: Robert Krátký <rkratky@redhat.com>
|
@rkratky, addressed your comment and made some more changes in my new commit. Also, you'll see a comment I made pointing out the many uses of passive voice. If you think it all looks fine, please go ahead and merge. Otherwise, I'm happy to continue to make changes. Thanks! |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jc-berger, johnmcollier, rkratky, rolfedh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Fixes #191 Define onboarding requirement
Taking content from John's devfile-registry docs repo: https://github.com/johnmcollier/registry-docs
I've left comments throughout the docs, asking questions to help direct peer reviewers' attention.
Thanks!
@johnmcollier or @GeekArthur would either of you take a look please to see how the devfile registry docs are looking?