Skip to content

Conversation

sudhakarnraju
Copy link

Currently newline is hardcoded as \n. PR addresses this for user to supply this parameter ( if not defaults to new line). Required since this varies between OS

@coveralls
Copy link

coveralls commented Aug 8, 2018

Coverage Status

Coverage remained the same at 94.805% when pulling b616cdf on sudhakarnraju:master into e7cdb55 on abdennour:master.

@jopek
Copy link

jopek commented Aug 15, 2018

@sudhakarnraju could you maybe reverse the formatting to the previous "not so pretty" style?
it's a bit difficult to see what places you introduced new functionality to when large portions of the code are reformatted.

@sudhakarnraju
Copy link
Author

@jopek reverted the formatting changes. Pls take look at the latest revisions. ( couple of tab spacing changes crept in, in spite of my best efforts, but shouldn't distract your review)

@mccabemj
Copy link
Collaborator

@sudhakarnraju can you please fix some of your formatting and style inconsistencies. In general this package uses single quotes. In multiple places you have changed to double quotes, where in others you have added new code with single quotes. As @jopek said, if you can revert your styling changes leaving just the core changes, we can review

@mccabemj mccabemj mentioned this pull request Oct 18, 2018
@mriccid mriccid force-pushed the master branch 2 times, most recently from 4c691cb to f129262 Compare February 5, 2019 05:12
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.

4 participants