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(plugin-ris) fix parse function #71

Merged
merged 3 commits into from
Oct 27, 2019

Conversation

soulchainer
Copy link
Contributor

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 my node version.

@codecov
Copy link

codecov bot commented Oct 27, 2019

Codecov Report

Merging #71 into v0.5 will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
packages/plugin-ris/src/ris.js 96% <100%> (+0.08%) ⬆️

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 92f3805...42ab486. Read the comment docs.

@soulchainer soulchainer force-pushed the fix-RIS-parse-function branch from 92fb6c4 to 177673d Compare October 27, 2019 17:15
@soulchainer soulchainer force-pushed the fix-RIS-parse-function branch from 177673d to 62ff97d Compare October 27, 2019 17:23
@soulchainer
Copy link
Contributor Author

soulchainer commented Oct 27, 2019

IDK what I have to do to improve the coverage here, if anything.
I understand that it's related to tests and that codecov (first time dealing with it) isn't happy because there is a decrease of coverage percentage, but I checked the tests for plugin-ris and I don't get them, how to improve the coverage... plus what I commented previously about tests doesn't even run OK in my machine, but dependencies problems. In this project there are several package.json and it seems that only installing dependencies of the plugin-ris package.json is not enought, neither also installing dependencies of citation-js, so I'm clueless, right now >_<.

@larsgw
Copy link
Member

larsgw commented Oct 27, 2019

Thank you!

To run the tests, you probably need to run npx lerna bootstrap. That installs (and links) the dependencies of the individual packages, while npm install, in this repository anyway, only installs generic devDependencies. After, yarn test and npm test should work. It's all in the new CONTRIBUTING.md, but the one in the v0.5 branch, not the one in master. That's my bad, sorry for the confusion.

Either way, you don't have to worry about the code coverage.

@larsgw larsgw changed the base branch from master to v0.5 October 27, 2019 21:17
line = line.trim()
if (!LINE_MATCH.test(line)) { lastEntry[lastTag] += line }
if (!LINE_MATCH.test(line)) {
if (lastEntry && lastTag) { lastEntry[lastTag] += ' ' + line.trim() }
Copy link
Contributor Author

@soulchainer soulchainer Oct 27, 2019

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?

Copy link
Member

@larsgw larsgw Oct 27, 2019

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.

@larsgw larsgw merged commit ffe3b07 into citation-js:v0.5 Oct 27, 2019
larsgw pushed a commit that referenced this pull request Oct 28, 2019
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.

2 participants