-
Notifications
You must be signed in to change notification settings - Fork 56
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
run/push: support "subargs" for forwarding to remote process. Fixes gh-732 #806
Conversation
2c6f1d4
to
3ed4a27
Compare
args[i] = arg.slice(0, arg.length - 1); | ||
eIndexOfSA = i; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: I deleted the line note because I'm adding the content as a comment in the source
6c38212
to
80840d8
Compare
@ebruchez wdyt? @HipsterBrown review? |
@@ -284,7 +284,8 @@ exports['Tessel.prototype.deploy'] = { | |||
test.expect(12); | |||
deployTestCode(this.tessel, { | |||
push: true, | |||
single: false | |||
single: false, | |||
subargs: [], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should pass without the subargs
property as well, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because this test runs code that exists at a point in which all default properties are resolved, so the subargs
property will always be present, either empty or with contents
Code looks good to me. A few questions:
|
Smoke test results:
Looks like a successful test! 👍 |
TBH, I'm not sure and this is an interesting question. I will do some tests and report back |
I've spent some time with this question and have determined that this patch is not an appropriate mechanism to shoe-horn other features into. I'm going to stop at "just subargs" here. |
@rwaldron Sounds good. I appreciate you taking the time to investigate that concept. LGTM 👍 |
…esselgh-732 Uses the "subarg" syntax: t2 run index.js [foo bar baz -a 1 -b 2 --named --key=value] Will result in process.argv on the deployed program: [ '/usr/bin/node', '/tmp/remote-script/index.js', 'foo', 'bar', 'baz', '-a', '1', '-b', '2', '--named', '--key=value' ] Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Uses the "subarg" syntax:
Will result in process.argv on the deployed program:
Smoke Test
(pull this branch locally)
Create a new project:
mkdir t2-732 && cd t2-732 && t2 init; npm install minimist --save
Open the
index.js
just now and replace the contents with:Run
t2 run index.js [foo bar baz -a 1 -b 2 --named --key=value]
Deployment should look something like this:
Signed-off-by: Rick Waldron waldron.rick@gmail.com