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

add first class rsh, better deal with args for proxy cmds like exec, rsh #81

Merged
merged 1 commit into from
Nov 16, 2017

Conversation

gabemontero
Copy link
Contributor

Fixes #79

@openshift/sig-developer-experience fyi

@gabemontero
Copy link
Contributor Author

[test]

@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 16, 2017

cmd.addAll(toStringArray(userArgs));

cmd.addAll(toStringArray(options));
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just ensuring that the "--" arg does not come before the "--loglevel" or other args being added?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah ... i should update the comment, forgot that the literal -- sequence could be a valid arg

what I was trying to convey was that things like --server, --namespace, --token, --loglevel in the wrong position would confuse oc rsh ... that is what was causing the problem .... moving those args prior to the verb works generically, and protects oc rsh

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, yeah a clarifying comment would help, otherwise lgtm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

comment clarified

@gabemontero
Copy link
Contributor Author

[merge]

@openshift-bot
Copy link

Evaluated for jenkins client plugin test up to 4cfdbd8

@openshift-bot
Copy link

Evaluated for jenkins client plugin merge up to 4cfdbd8

@openshift-bot
Copy link

openshift-bot commented Nov 16, 2017

continuous-integration/openshift-jenkins-client-plugin/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_jenkins_client_plugin/7/) (Base Commit: 5c823bc) (PR Branch Commit: 4cfdbd8) (Image: devenv-rhel7_8)

@openshift-bot
Copy link

continuous-integration/openshift-jenkins-client-plugin/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_jenkins_client_plugin/20/) (Base Commit: 5c823bc) (PR Branch Commit: 4cfdbd8)

@openshift-bot openshift-bot merged commit 881f47c into openshift:master Nov 16, 2017
@gabemontero gabemontero deleted the add-rsh branch November 16, 2017 22:05
gabemontero pushed a commit to jenkinsci/openshift-client-plugin that referenced this pull request Nov 20, 2017
@ghost ghost mentioned this pull request Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

None yet

4 participants