search

wikimedia / html-metadata

MetaData html scraper and parser for Node.js (supports Promises and callback style)
MIT License
138 stars 44 forks source link

Add Eprints tag scraping #32

Closed Scimonster closed 8 years ago

Scimonster commented 8 years ago

Adds parseEprints function to scrape eprints metadata. Bump version number *Minor documentation fixes

https://phabricator.wikimedia.org/T118634

Bug: T118634

mvolz commented 8 years ago

Overall great job, just some nitpicky things. Also re:commit messages, there are mediawiki guidelines somewhere that we adhere to; but basically, just be specific; if the change is too minor to mention specifically you can just not mention it.

mvolz commented 8 years ago

https://m.mediawiki.org/wiki/Gerrit/Commit_message_guidelines

Scimonster commented 8 years ago

Those failures don't seem to have anything to do with me.

mvolz commented 8 years ago

One isn't, that's a url that keeps changing upstream... easy to fix, you can just fix the url (or I or someone else might get to it before you- we really need to use a more stable test!)

The other test only fails with the introduction of c2952ef, so it's probably how you changed the tests. If the error message doesn't make sense, sometimes that happens with promises/mocha...

Also it would be great if you could put the version number in where you add the functionality, that way we don't have a mismatch. (README too although less important- but you might find it easier to do a softt reset back to HEAD~2 anyway).You can actually amend commit on github git commit --amend but you have to force push (-F) and it does it silently (changes will appear with no notification).