Laverna / laverna

Laverna is a JavaScript note taking application with Markdown editor and encryption support. Consider it like open source alternative to Evernote.
https://laverna.cc/index.html
Mozilla Public License 2.0
9.18k stars 801 forks source link

major linting overhaul #964

Closed daed closed 6 years ago

daed commented 6 years ago

I spent some time today trying to fix the linter errors so that the travis build would show up successful again.

I fixed all of the ones I knew how to, and then downgraded the others to "warnings" in the eslintrc file.

Obviously this isn't great as far as review goes because I touched a ton of files in the process, but from what I can tell, I didn't break anything.

daed commented 6 years ago

It looks like the node 6 test passed. Stable crashed though.

Maybe something related to this? https://github.com/nodejs/node/issues/20285

daed commented 6 years ago

The above changes work for the tests including node 8.11.3.

Fixing this to work on node 10.7.0 is going to be a huge undertaking. The core issue here is that there are packages (gulp 3.x, dependent on vinyl-fs which is dependent on glob-watcher which is dependent on gaze which is dependent on ... until you get to an ancient version of graceful-fs) that use unsupported/undocumented things from the natives package, and in node 10 they've restricted access to a lot of the api that was exposed though that package.

There's two workarounds I've found:

I don't know which way is the better way to go. For my own purposes, I'll probably just do the change for natives 1.1.4 and submit a PR for it anyway.

daed commented 6 years ago

This needed some extra work. I managed to get the node 10 build to succeed, but I had to turn off some of the tests in the meanwhile. I'm not 100% sure whether it's the tests or the code itself that is failing, but functionality appears good in the program itself.

I need to do some more work before I'm comfortable with a PR on this.