Skip to content

Conversation

@dluxemburg
Copy link
Contributor

Adds the ability to set current working directory for command, handled the same was as Node's child_process. Test included.

@dluxemburg
Copy link
Contributor Author

I left it as null because that's what the Node docs indicated the default was for child_process, so I figured that's what would make the fewest assumptions on top of the underlying implementation. I can change it to . in the pull request if you'd prefer.

@sun-zheng-an
Copy link
Owner

I prefer ., because I don't want to leak to implementation details that gulp-shell is built on top of child_process.exec().

@sun-zheng-an
Copy link
Owner

process.cwd() may be a better choice, what do you think?

@dluxemburg
Copy link
Contributor Author

I'm not certain how they'd behave differently. It seems like process.cwd() might be best because it will reliably make the command share the working directory of the parent process (almost certainly gulp itself). I'll change the commit now.

@sun-zheng-an
Copy link
Owner

You forgot to modify the README.md.

Could you squash your PR into a single commit?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2c49402 on dluxemburg:cwd-options into 3ff1cf1 on sun-zheng-an:master.

@dluxemburg
Copy link
Contributor Author

Done!

On Sunday, February 23, 2014 at 9:37 PM, sun-zheng-an wrote:

You forgot to modify the README.md (http://README.md).
Could you squash your PR into a single commit?


Reply to this email directly or view it on GitHub (#2 (comment)).

sun-zheng-an added a commit that referenced this pull request Feb 24, 2014
add options.cwd to set current working directory for command
@sun-zheng-an sun-zheng-an merged commit 1e8ae38 into sun-zheng-an:master Feb 24, 2014
sun-zheng-an added a commit that referenced this pull request Sep 1, 2017
add options.cwd to set current working directory for command
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.

3 participants