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

Deprecate process -v/--value in favor of -p/--param #12001

Merged
merged 1 commit into from
Nov 23, 2016
Merged

Deprecate process -v/--value in favor of -p/--param #12001

merged 1 commit into from
Nov 23, 2016

Conversation

jupierce
Copy link
Contributor

Fixes #8577
ptal @bparees @csrwng
Assuming this is acceptable, I will make a corresponding PR for openshift-docs.

@jupierce jupierce changed the title Depcrecate process -v/--value in favor of -p/--param Deprecate process -v/--value in favor of -p/--param Nov 22, 2016
@bparees
Copy link
Contributor

bparees commented Nov 22, 2016

i'm certainly in favor of it. @fabianofranz any issues w/ this?

@bparees
Copy link
Contributor

bparees commented Nov 22, 2016

@openshift/api-review ptal

@smarterclayton
Copy link
Contributor

Test case showing deprecated behavior works.

API change approved.

@jupierce
Copy link
Contributor Author

[test]

@fabianofranz
Copy link
Member

any issues w/ this?

Works for me.

os::cmd::expect_success "oc process -f '${required_params}' --value=required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' -v required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' --param=required_param=someval"
os::cmd::expect_success "oc process -f '${required_params}' -p required_param=someval | oc create -f -"
# parameters with multiple equal signs are OK
os::cmd::expect_success "oc process -f '${required_params}' required_param=someval=moreval | oc create -f -"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm interested in this test both -v and -p

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@deads2k 27067a9 ptal

that works, thanks.

@jupierce
Copy link
Contributor Author

@bparees
Copy link
Contributor

bparees commented Nov 23, 2016

[merge]

@jupierce
Copy link
Contributor Author

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 27067a9

@bparees
Copy link
Contributor

bparees commented Nov 23, 2016

[merge]

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 27067a9

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11675/) (Base Commit: 6f81303)

@openshift-bot
Copy link
Contributor

openshift-bot commented Nov 23, 2016

continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/11679/) (Base Commit: 76a5795) (Image: devenv-rhel7_5413)

@openshift-bot openshift-bot merged commit 33a69c5 into openshift:master Nov 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants