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

remove show-all option description since it is now defaulted to true #7689

Closed
wants to merge 2 commits into from

Conversation

WanLinghao
Copy link
Contributor

show-all is now defaulted to true and deprecated:
kubernetes/kubernetes#60210
It will be inert in 1.11 and removed in a future release.
This patch fix related description on jobs doc.

        It will be inert in 1.11 and removed in a future release.
	This patch fix related description on jobs doc.
	modified:   docs/concepts/workloads/controllers/jobs-run-to-completion.md
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 9, 2018
@k8s-ci-robot k8s-ci-robot requested review from erictune and soltysh March 9, 2018 05:27
@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Mar 9, 2018

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 57b4937

https://deploy-preview-7689--kubernetes-io-master-staging.netlify.com

@bradtopol bradtopol self-assigned this Mar 9, 2018
@bradtopol
Copy link
Contributor

Overall this change looks very good to me. My only question is it worth adding a callout note mentioning that the show all option is no longer needed since it has been deprecated and defaulted to true. @zacharysarah Thoughts on this?

@soltysh
Copy link
Contributor

soltysh commented Mar 9, 2018

@bradtopol we should remove any references to --show-all from all of the docs. I haven't checked if there are more, @WanLinghao can you double check please? The option is deprecated, so it won't appear anywhere, the docs should be the same. The current change itself lgtm.

@zacharysarah
Copy link
Contributor

@bradtopol This sounds like a great candidate for the release notes rather than the docs.

/cc @WanLinghao @nickchase

@k8s-ci-robot
Copy link
Contributor

@zacharysarah: GitHub didn't allow me to request PR reviews from the following users: nickchase, WanLinghao.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

@bradtopol This sounds like a great candidate for the release notes rather than the docs.

/cc @WanLinghao @nickchase

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.

@soltysh
Copy link
Contributor

soltysh commented Mar 9, 2018

I'd say both, actually.

@nickchase
Copy link

nickchase commented Mar 10, 2018 via email

@WanLinghao
Copy link
Contributor Author

@bradtopol @soltysh @zacharysarah thanks for your reply。I will do a thorough check

@tengqm
Copy link
Contributor

tengqm commented Mar 10, 2018

should this be proposed against the release-1.10 branch?

	modified:   docs/tasks/job/parallel-processing-expansion.md
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: zacharysarah

Assign the PR to them by writing /assign @zacharysarah in a comment when ready.

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 added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 12, 2018
@WanLinghao
Copy link
Contributor Author

WanLinghao commented Mar 12, 2018

I find other three places that have --show-all description.
I am not sure if two of them should be amended.
1.https://kubernetes.io/docs/reference/generated/kubefed_join/
Description here involved in kubefed join command which I have checked. It use --show-all option without any signs that it will be deprecated. So may I launch a PR to make it deprecated as kubectl do and then amend docs?

2.https://kubernetes.io/docs/reference/generated/kubectl/kubectl-commands
This doc contains many reference of --show-all option. Are they supposed to refreshed by some kind of script?

3.https://kubernetes.io/docs/tasks/job/parallel-processing-expansion/
References here has been amended by a new commit
@soltysh @bradtopol @zacharysarah

@WanLinghao
Copy link
Contributor Author

so..can anyone just approve this patch~
@soltysh @bradtopol @zacharysarah

@bradtopol
Copy link
Contributor

bradtopol commented Mar 15, 2018

@WanLinghao please retarget the PR from the master branch to the release-1.10 branch. Getting close to cutoff so need @Bradamant3 input on what to do with this at this stage. Thank you again for your contribution

@bradtopol
Copy link
Contributor

Upon further investigation by @Bradamant3 we have already had two PRs submitted to fix this for 1.10 so I am going to close this. See #7574 Thank you.

@bradtopol bradtopol closed this Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants