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

Fix InlineArrayTransformer adding extra newlines when maintaining indentation [WIP] #114

Merged
merged 3 commits into from
Nov 22, 2017
Merged

Fix InlineArrayTransformer adding extra newlines when maintaining indentation [WIP] #114

merged 3 commits into from
Nov 22, 2017

Conversation

ibrahima
Copy link
Contributor

The latter test tests behavior that I think should be correct, but it
fails because it is not the current behavior.

The latter test tests behavior that I think should be correct, but it
fails because it is not the current behavior.
@codecov-io
Copy link

codecov-io commented Sep 12, 2017

Codecov Report

Merging #114 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #114   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          23     23           
  Lines         149    149           
=====================================
  Hits          149    149
Impacted Files Coverage Δ
...c/inlineArrayTransformer/inlineArrayTransformer.js 100% <100%> (ø) ⬆️

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 9295ee1...cb95d00. Read the comment docs.

@fatfisz
Copy link
Collaborator

fatfisz commented Nov 22, 2017

Hi, what a good catch!

Answering your question from #113: I think using a negation in this case is just fine because it should be future-proof.

@fatfisz
Copy link
Collaborator

fatfisz commented Nov 22, 2017

@ibrahima I don't have permissions to re-run the build, but OTOH you could add a commit that adds you to contributors in package.json. Could you please do that and then we could go ahead with the merge if everything's green?

@ibrahima
Copy link
Contributor Author

I just pushed a merge commit; I feel weird adding myself to contributors in package.json since it's a small patch, and not all the existing contributors on Github are in there. But anyway, CI should run again now.

@fatfisz
Copy link
Collaborator

fatfisz commented Nov 22, 2017

If that's how you feel, then ok. If you change your mind though, don't hesitate to get back to us - you actually found the root cause and spent some time on fixing the problem, which is much more than most people do. If this is not worthy a mention in package.json, then I don't know what is 😉

@fatfisz fatfisz merged commit 6023b75 into zspecza:master Nov 22, 2017
@ibrahima ibrahima deleted the ibrahim/inline-transformer-array-excess-newlines branch November 22, 2017 23:56
fatfisz added a commit that referenced this pull request Nov 23, 2017
@fatfisz
Copy link
Collaborator

fatfisz commented Nov 24, 2017

Released in v1.5.0!

@ibrahima
Copy link
Contributor Author

Thanks!

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.

None yet

3 participants