Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

feat(bin): two bins (shell, non-shell) #104

Merged
merged 2 commits into from Apr 18, 2017
Merged

Conversation

hgwood
Copy link
Collaborator

@hgwood hgwood commented Apr 9, 2017

Revert the default bin (cross-env) to its v3 behavior (not using the shell option). Add a new
(cross-env-shell) which uses the shell option.

BREAKING CHANGE: Scripts using quotes or escape sequences will see a difference in behavior.
Switching to the second bin should resolve any issue.

Closes #99.

See prior discussion in #99 et and #102.

@codecov-io
Copy link

codecov-io commented Apr 9, 2017

Codecov Report

Merging #104 into next will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@         Coverage Diff         @@
##           next   #104   +/-   ##
===================================
  Coverage   100%   100%           
===================================
  Files         3      3           
  Lines        45     42    -3     
===================================
- Hits         45     42    -3
Impacted Files Coverage Δ
src/index.js 100% <ø> (ø) ⬆️
src/variable.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8d4ec1d...e295a22. Read the comment docs.

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you! Could we add docs in the README? We could really use an improvement the I think.

@kentcdodds kentcdodds changed the base branch from master to next April 13, 2017 16:39
@kentcdodds
Copy link
Owner

I've moved this to go to the next branch so we can group this with another breaking change 👍

@hgwood
Copy link
Collaborator Author

hgwood commented Apr 14, 2017

Thanks. I'll get to work on the documentation over the week-end.

Revert the default bin (cross-env) to its v3 behavior (not using the shell option). Add a new
(cross-env-shell)  which uses the shell option.

BREAKING CHANGE: Scripts using quotes or escape sequences will see a difference in behavior.
Switching to the second bin should resolve any issue.

Closes kentcdodds#99.
@hgwood
Copy link
Collaborator Author

hgwood commented Apr 18, 2017

Documentation updated. What do you think?

Copy link
Owner

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Just one little typo, then we're good. Feel free to merge once you've fixed the typo 👍

README.md Outdated
second one uses the `shell` option from Node's `spawn`.

The main use case for `cross-env-shell` is when your need an environment
vartiable to be set across an entire inline shell script, rather than just one
Copy link
Owner

Choose a reason for hiding this comment

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

vartiable

should be:

variable

}
}
```

The rule of thumb is: if you want to pass to `cross-env` a command that
Copy link
Owner

Choose a reason for hiding this comment

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

Great to add this. Thanks 👍

@kentcdodds kentcdodds merged commit 9a1c830 into kentcdodds:next Apr 18, 2017
@kentcdodds
Copy link
Owner

I fixed the typo. I'll release a beta for the next branch 👍

@hgwood hgwood deleted the two-bins branch April 19, 2017 14:41
kentcdodds pushed a commit that referenced this pull request May 11, 2017
Revert the default bin (cross-env) to its v3 behavior (not using the shell option). Add a new
(cross-env-shell)  which uses the shell option.

BREAKING CHANGE: Scripts using quotes or escape sequences will see a difference in behavior.
Switching to the second bin should resolve any issue.

Closes #99.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants