Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

freitagbr
Copy link
Contributor

Splits and joins on newlines only.

Fixes #645 .

@freitagbr freitagbr added the fix Bug/defect, or a fix for such a problem label Mar 14, 2017
@freitagbr freitagbr requested a review from nfischer March 14, 2017 03:33
@codecov-io
Copy link

Codecov Report

Merging #690 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #690      +/-   ##
==========================================
- Coverage   94.67%   94.67%   -0.01%     
==========================================
  Files          33       33              
  Lines        1183     1182       -1     
==========================================
- Hits         1120     1119       -1     
  Misses         63       63
Impacted Files Coverage Δ
src/uniq.js 100% <ø> (ø)
src/grep.js 100% <100%> (ø)
src/sed.js 100% <100%> (ø)
src/sort.js 96.42% <100%> (-0.13%)

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 b9201eb...36e7e66. Read the comment docs.

@nfischer
Copy link
Member

@freitagbr can you see what the issue is with code coverage? If we're uncovering any lines, we should see if we can add tests.

@freitagbr
Copy link
Contributor Author

The diff is +7/-9, so overall -2, so the coverage is going to decrease. No lines are becoming uncovered.

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.

LGTM, but should we consider this a breaking change? We didn't need to change any tests, but this change would be visible to users operating on CRLF files.

@freitagbr
Copy link
Contributor Author

I think it could be breaking. Just to be safe, let's change the target branch to dev so this can land in v0.8.

@freitagbr freitagbr changed the base branch from master to dev March 15, 2017 19:19
@freitagbr freitagbr changed the title fix(grep, sed, sort, uniq): Split only on newline characters WIP: fix(grep, sed, sort, uniq): Split only on newline characters Mar 15, 2017
@freitagbr freitagbr changed the title WIP: fix(grep, sed, sort, uniq): Split only on newline characters fix(grep, sed, sort, uniq): Split only on newline characters Mar 16, 2017
@nfischer nfischer merged commit a7ee6a2 into dev Mar 16, 2017
@nfischer nfischer deleted the split-newline branch March 16, 2017 08:08
freitagbr added a commit that referenced this pull request Jun 7, 2017
* Split on newlines only

* Only split lines if need be

* Clarify code by making use of Array.prototype.reduce
@nfischer nfischer mentioned this pull request Jun 7, 2017
nfischer pushed a commit that referenced this pull request Jun 7, 2017
* Split on newlines only

* Only split lines if need be

* Clarify code by making use of Array.prototype.reduce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug/defect, or a fix for such a problem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.