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

oc new-app: allow 'dot' in ENV variable names #19688

Conversation

wozniakjan
Copy link
Contributor

fixes: #8771

reuses validators from kubernetes/kubernetes#48986

ptal @openshift/sig-developer-experience

@wozniakjan wozniakjan requested a review from bparees May 11, 2018 12:15
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 11, 2018
@bparees
Copy link
Contributor

bparees commented May 11, 2018

looks like this is breaking some existing tests that probably need to be updated: https://openshift-gce-devel.appspot.com/build/origin-ci-test/pr-logs/pull/19688/test_pull_request_origin_cmd/13529/ (the one that is nil-ptr'ing is particularly concerning, that one may need changes in the actual code, not just the test)

should also introduce some new tests that will let us know if we regress this support in the future.

otherwise lgtm.

@wozniakjan wozniakjan changed the title oc new-app: allow 'dot' in ENV variable names [WIP] oc new-app: allow 'dot' in ENV variable names May 11, 2018
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2018
@wozniakjan wozniakjan force-pushed the issue8771/oc_new-app/env_var_names_with_dots branch 8 times, most recently from 5693a5d to 0d9b36b Compare May 15, 2018 12:38
@wozniakjan wozniakjan force-pushed the issue8771/oc_new-app/env_var_names_with_dots branch from 0d9b36b to d49e16d Compare May 15, 2018 12:40
@wozniakjan
Copy link
Contributor Author

/retest
FAILURE: SYNC REPOSITORIES

@wozniakjan
Copy link
Contributor Author

/retest
#19719

@bparees
Copy link
Contributor

bparees commented May 15, 2018

/approve
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 15, 2018
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, wozniakjan

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 15, 2018
@wozniakjan
Copy link
Contributor Author

/test unit
#19719

@wozniakjan wozniakjan changed the title [WIP] oc new-app: allow 'dot' in ENV variable names oc new-app: allow 'dot' in ENV variable names May 16, 2018
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 16, 2018
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit ef7e6c7 into openshift:master May 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/cli lgtm Indicates that a PR is ready to be merged. sig/developer-experience size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OC CLI: Environment variable with a DOT in them.
5 participants