Closed abought closed 7 years ago
Ran through manual regression testing on all current sample plots, and working as expected. [1] Will go ahead and merge, but not cut a release for this.
@pjvandehaar , these are the tiny changes we discussed last week. I don't expect it to affect your work, but let me know if PheWeb has trouble.
[1] I did see some issues with p-value tooltips not appearing in the PheWAS forest plot, but it also occurs on master; fix is out of scope for this PR. https://github.com/statgen/locuszoom/blob/master/examples/phewas_forest.png
Purpose
Add ESLint to the default test command, to help catch common errors. In particular, I've updated the rules and code to de-emphasize the use of magic type coercion (
== in favor of ===
), which should help prevent edge case bugs as we explore more custom plots in the future. Any tests that relied on this behavior have been fixed.Also added an npm5 lockfile to the repository, to make dependencies more deterministic. (details)
Lastly, updated travis to use caching, which should enable faster builds. Travis will now require that all linting pass as part of any pull request.
Tips
eslint --fix .
will automatically reformat for simple conventions (like indents), to make usage less burdensome.npm run eslint
will save you from having to type the full path to the executable.npm test
to subject your code to the exact rules that Travis uses. I have omitted eslint from the gulp task intentionally, to keep thegulp watch
workflow fluid.Testing notes
All mocha tests passing. As this is a first PR to this repo, I'll do a pass of manual testing on Monday, and then may recruit someone with custom LZ plots for manual verification.
(Custom or third party plots are probably a bit more likely to test the limits of what plots do, or to use the code in novel or unique ways)