Uh oh!
There was an error while loading. Please reload this page.
#884: Cleanup examples folder - #938
Conversation
micw523
left a comment
There was a problem hiding this comment.
I didn’t get through all of it, but some of the changes introduced here do warrant more discussion / more carefully thought over. Also, large PRs can be very hard to review.
Uh oh!
There was an error while loading. Please reload this page.
| # shortNames: | ||
| # - ct | ||
| """ | ||
| Uses a Custom Resource Definition (CRD) to create a custom object, in this case |
There was a problem hiding this comment.
Why is this first sentence needed?
There was a problem hiding this comment.
Not sure what you're asking? Let me know if you'd prefer something else.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
micw523
commented
Sep 5, 2019
CI is failing - please address the code style suggestions by the CI |
scottilee
commented
Sep 5, 2019
@micw523 I fixed all the code style suggestions related to the files touched in this PR but there's additional ones about unrelated files. |
micw523
commented
Sep 5, 2019
E501 errors are non-blocking; the problems are in pod_namespace_watch.py, examples/pod_exec.py, pod_config_list.py, examples/custom_object.py. Try using autopep8 with aggressive level 2 on those files. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
micw523
left a comment
There was a problem hiding this comment.
The create_deployment.py file is renamed to deployment_create_yaml.py. I prefer the previous name b/c this file is not using the create_from_yaml utlity but calling the API directly.
| @@ -1,367 +0,0 @@ | |||
| { | |||
There was a problem hiding this comment.
This is a little different from the create_deployment file since it is also using rollback and rollingupdate.
There was a problem hiding this comment.
Restored. I also changed deployment_create_yaml.py to deployment_create.py
There was a problem hiding this comment.
Are these ipynb file auto-generated? Do they affect the document website? @micw523
There was a problem hiding this comment.
This one is not automatically generated and was written by someone a long time ago. However, I believe this one is a little out of date for using beta API (?).
Uh oh!
There was an error while loading. Please reload this page.
scottilee
commented
Sep 11, 2019
Renamed files: examples/create_deployment.py → examples/deployment_create.py |
roycaihw
commented
Sep 11, 2019
example1 and in_cluster_config are under different assumptions and use different config loading methods. Could you keep both of them and rename example1 to out_of_cluster_config (similar to what client-go has)? |
scottilee
commented
Sep 13, 2019
@roycaihw done, and I rebased. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
roycaihw
commented
Sep 13, 2019
One major questions about unexpected example content being changed (oversight during copy-pasting?): #938 (comment). Otherwise looks pretty good. Thanks @scottilee! /approve @micw523 has lgtm |
k8s-ci-robot
commented
Sep 13, 2019
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: roycaihw, scottilee The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Yeah, lgtm to me as long as the last round of comments are addressed. |
micw523
commented
Sep 14, 2019
/lgtm |
Update examples that were moved in kubernetes-client#938 and fix title underlines (docutils cares about this stuff). Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Fixes#884