Skip to content

Remove prompts from commented examples#8454

Merged
openshift-bot merged 1 commit intoopenshift:masterfrom
rhcarvalho:bash-examples-syntax
May 12, 2016
Merged

Remove prompts from commented examples#8454
openshift-bot merged 1 commit intoopenshift:masterfrom
rhcarvalho:bash-examples-syntax

Conversation

@rhcarvalho
Copy link
Copy Markdown
Contributor

Following the upstream style changes from Kubernetes.

Based on kubernetes/kubernetes#13719 and kubernetes/kubernetes#22177.

@rhcarvalho
Copy link
Copy Markdown
Contributor Author

@Kargakis PTAL

@0xmichalis
Copy link
Copy Markdown
Contributor

@smarterclayton I would like us to have this for 1.2 for parity with kubectl

@0xmichalis
Copy link
Copy Markdown
Contributor

cc: @fabianofranz

@smarterclayton
Copy link
Copy Markdown
Contributor

It's likely too late to make a change like this - it's not a must have, it's just a nice to have.

@fabianofranz
Copy link
Copy Markdown
Member

You will have to hack/update-generated-docs.sh but apart that LGTM.

There are no potentially breaking changes here so if kubernetes/kubernetes#22177 landed before the version of kube we have then my vote is to have this to 1.2, but it's up to @smarterclayton to decide.

@rhcarvalho rhcarvalho force-pushed the bash-examples-syntax branch from 081e5fc to 9a3ff7c Compare April 11, 2016 14:26
@rhcarvalho
Copy link
Copy Markdown
Contributor Author

Thanks @fabianofranz! I've run hack/update-generated-docs.sh and updated my commit.

@smarterclayton
Copy link
Copy Markdown
Contributor

Given size of change and lateness of schedule, we should only be merging
p0's or build/test fixes.

On Mon, Apr 11, 2016 at 10:27 AM, Rodolfo Carvalho <notifications@github.com

wrote:

Thanks @fabianofranz https://github.com/fabianofranz! I've run
hack/update-generated-docs.sh and updated my commit.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#8454 (comment)

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 21, 2016
@rhcarvalho rhcarvalho force-pushed the bash-examples-syntax branch from 9a3ff7c to f411bca Compare April 25, 2016 13:58
@rhcarvalho
Copy link
Copy Markdown
Contributor Author

Rebased.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Does anybody know if this should be uncommented? The TODO was added in #4299, last August.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, I think we can uncomment those now. Mind adding test cases for them, if we don't have already? (which most probably is the case:) )

@rhcarvalho rhcarvalho force-pushed the bash-examples-syntax branch from f411bca to 5683900 Compare April 25, 2016 14:19
@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 25, 2016
@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels May 6, 2016
@0xmichalis
Copy link
Copy Markdown
Contributor

@rhcarvalho please rebase so we can move forward with this.

@rhcarvalho rhcarvalho force-pushed the bash-examples-syntax branch from 5683900 to 325a741 Compare May 12, 2016 13:33
@rhcarvalho
Copy link
Copy Markdown
Contributor Author

@Kargakis rebased.

@0xmichalis
Copy link
Copy Markdown
Contributor

LGTM [merge]

@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 12, 2016
@openshift-bot
Copy link
Copy Markdown
Contributor

openshift-bot commented May 12, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_origin/5896/) (Image: devenv-rhel7_4175)

Following the upstream style changes from Kubernetes.
@rhcarvalho rhcarvalho force-pushed the bash-examples-syntax branch from 325a741 to af2094c Compare May 12, 2016 14:42
@rhcarvalho
Copy link
Copy Markdown
Contributor Author

@Kargakis I fixed the generated docs, was missing a re-run after I replaced some '$ ' in new code after the rebase.

@rhcarvalho rhcarvalho mentioned this pull request May 12, 2016
@0xmichalis
Copy link
Copy Markdown
Contributor

[test]

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin test up to af2094c

@openshift-bot
Copy link
Copy Markdown
Contributor

continuous-integration/openshift-jenkins/test FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/3779/)

@0xmichalis
Copy link
Copy Markdown
Contributor

#8865 and redis image flakes

[merge]ing

@openshift-bot
Copy link
Copy Markdown
Contributor

Evaluated for origin merge up to af2094c

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants