search

Shikkic / gh-scrape

module to scrape public user stats off github
https://www.npmjs.com/package/gh-scrape
MIT License
9 stars 4 forks source link

renegade code review #1

Closed shrugs closed 7 years ago

shrugs commented 8 years ago

requiring underscore and not using it? Don't forget to remove it from package.json

Does the scrapeContributionData function have a bug in it on this line? the data variable doesn't exist in this scope. should data be html instead? Maybe I'm missing something, but it doesn't look like scrapeContributionData will ever work correctly.

Your separation of concerns for each function is :+1:

Should return Error; on this line be return error;, to re-use the request's error, instead of returning the Error class?

Shikkic commented 8 years ago

I think the underscore was used for a previous build, I think I was going to use it for some cleaner iterative functions for pulling out most recent commit data.

Oh my, I didn't even notice that, I have been using this module for bitBar and have seen this error. I thought it was a connectivity issue. However I can still get it to work sometimes, so thats weird...

Ahh that last bit also makes sense as well.

Shikkic commented 8 years ago

I don't like the error handling I have for this. It will at most check twice. I haven't really found module that allows to request and retry based on a condition of the value. Any thoughts?