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

Merge dev into master #731

Merged
merged 8 commits into from
Jun 7, 2017
Merged

Merge dev into master #731

merged 8 commits into from
Jun 7, 2017

Conversation

freitagbr
Copy link
Contributor

Like #650 but with the merge conflicts resolved.

freitagbr and others added 7 commits June 6, 2017 20:15
* Add newline to output of echo

* Add test
* Split on newlines only

* Only split lines if need be

* Clarify code by making use of Array.prototype.reduce
* Add newline to output of echo (#557)

* Add newline to output of echo

* Add test

* Throw an error if the options string does not start with '-' (#615)

* Throw an error if the options string does not start with '-'

* Add test

* Change message grammar

* Add -n option to echo

* Fix null argument issue

* Add -n tests

* Add documentation

* Add -en escaped character test

* Add function to parse options for echo

* Use parseOptions to parse echo options

* Simplify control flow

* parseOptions throws now

* Allow null to be echoed

* Prevent echo stderr on unrecognized option

* Add test to check stderr of returned value

* Use consistent variable name

* Change test message, leave TODO about console output
* Add stdout/stderr test mocks

* Mock stdout/stderr during echo tests

* Fix lint issues

* Use 'use strict'

* Re-implement mocks as a prototype

* Implement mocks as a single-instance

* Remove redundant test

* Create mocked stdout/stderr.write methods once
@freitagbr freitagbr added breaking Breaking change high priority labels Jun 7, 2017
@freitagbr freitagbr added this to the v0.8.0 milestone Jun 7, 2017
@freitagbr freitagbr requested a review from nfischer June 7, 2017 03:25
@nfischer
Copy link
Member

nfischer commented Jun 7, 2017

Here's the list of PRs that found their way onto dev branch. We'll want to make sure all of these are reflected on this new dev branch.

* Deprecate common.getUserHome, advise using os.homedir instead

* Remove common.getUserHome
@codecov-io
Copy link

codecov-io commented Jun 7, 2017

Codecov Report

Merging #731 into master will increase coverage by 1.84%.
The diff coverage is 97.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #731      +/-   ##
==========================================
+ Coverage   94.81%   96.65%   +1.84%     
==========================================
  Files          33       33              
  Lines        1254     1227      -27     
==========================================
- Hits         1189     1186       -3     
+ Misses         65       41      -24
Impacted Files Coverage Δ
src/tempdir.js 100% <ø> (ø) ⬆️
src/uniq.js 100% <ø> (ø) ⬆️
src/sed.js 100% <100%> (ø) ⬆️
src/echo.js 100% <100%> (+62.5%) ⬆️
src/common.js 98.29% <100%> (-0.64%) ⬇️
src/cd.js 100% <100%> (ø) ⬆️
src/sort.js 96.96% <100%> (-0.09%) ⬇️
src/grep.js 100% <100%> (ø) ⬆️
src/exec.js 92.59% <91.66%> (+17.35%) ⬆️

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 bbd8178...c423efd. Read the comment docs.

@nfischer
Copy link
Member

nfischer commented Jun 7, 2017

Expanded list:

@nfischer
Copy link
Member

nfischer commented Jun 7, 2017

#546 and #649 were the same PR (we lost the first one due to rebasing mistakes). "Fix broken test" contains #615.

@nfischer
Copy link
Member

nfischer commented Jun 7, 2017

It looks like "Echo stdout #677" contains #590

Copy link
Member

@nfischer nfischer left a comment

Choose a reason for hiding this comment

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

The coverage drops are reasonable as long as we work to get those lines covered in future PRs. I just released v0.7.8 (last main patch to v0.7.x), so I'm going to go ahead and land this.

@freitagbr thanks for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants