search

benfred / venn.js

Area proportional Venn and Euler diagrams in JavaScript
MIT License
1.04k stars 218 forks source link

Run a JavaScript linter on the code #66

Closed tsullivan closed 8 years ago

tsullivan commented 8 years ago

Someone copied venn.js into a project and it started getting covered with ESLint that we use in our test pipeline. The linter pointed out that there's an undefined variable here: https://github.com/benfred/venn.js/blob/master/venn.js#L935

We'll import the module with npm as should have been done initially, but just want to point out that undefined variable could be a problem. Maybe it's not right now because the classicMDSLayout function gets never gets called?

benfred commented 8 years ago

Thanks for the heads up, I should probably just remove this function from the package. I'll at least patch this on the weekend so it passes eslint w/ no-undef enabled).

I think I only left it so I could easily run the comparison perf tests here http://www.benfrederickson.com/better-venn-diagrams/ (which show that the classic MDS code just doesn't work that well).

benfred commented 8 years ago

I've removed that function here: https://github.com/benfred/venn.js/commit/7b1f8c37b720abad4d8890b84d7f1db53f3ddd22

I also added a basic eslint config file there too, and verified the code in src/*.js passes (though didn't yet add to travis-ci, which still only runs jshint and the unittests).