-
Notifications
You must be signed in to change notification settings - Fork 44
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(plugin-ris) fix parse function #71
Conversation
Codecov Report
@@ Coverage Diff @@
## v0.5 #71 +/- ##
==========================================
+ Coverage 88.12% 88.13% +<.01%
==========================================
Files 74 74
Lines 1541 1542 +1
==========================================
+ Hits 1358 1359 +1
Misses 183 183
Continue to review full report at Codecov.
|
92fb6c4
to
177673d
Compare
177673d
to
62ff97d
Compare
IDK what I have to do to improve the coverage here, if anything. |
Thank you! To run the tests, you probably need to run Either way, you don't have to worry about the code coverage. |
packages/plugin-ris/src/ris.js
Outdated
line = line.trim() | ||
if (!LINE_MATCH.test(line)) { lastEntry[lastTag] += line } | ||
if (!LINE_MATCH.test(line)) { | ||
if (lastEntry && lastTag) { lastEntry[lastTag] += ' ' + line.trim() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @larsgw. One question: why is it needed to trim()
line here when line
already comes trimmed from line 52
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second one can be removed, the line needs to be trimmed for LINE_MATCH
mainly. That second one comes from eba2bfe which hasn't landed on master
yet.
Closes #66
I tried to run
yarn test
of this project, but in fails big time... I think this could be related with this being a multirepo and all that stuff. Any insight (just because I'm curious) about this?I even used
nvm use v6
, for using the proper version of node this project states it supports, just in case it was because of mynode
version.