overleaf / spelling

The backend spellcheck API that performs spell checking for Overleaf
GNU Affero General Public License v3.0
9 stars 17 forks source link

Initial decaffeination #24

Closed mserranom closed 5 years ago

mserranom commented 5 years ago

Initial decaffeination of spelling module.

It replaces https://github.com/overleaf/spelling/pull/23, which showed lots of issues after node version bump and build scripts update.

Once this initial decaffeination is merged and tested, next step is update Node version from 6 to 8/10 to support https://github.com/overleaf/error-type library. This requires removing v8 profiler, which is incompatible with node > 6.

mserranom commented 5 years ago

@timothee-alby I think /src is a web convention, it's not enforced by build scripts and other decaff work is done using /js (as in https://github.com/overleaf/chat/pull/17/files)

40thieves commented 5 years ago

I think /src is a web convention, it's not enforced by build scripts and other decaff work is done using /js (as in https://github.com/overleaf/chat/pull/17/files)

Sorry I did notice this when we were looking at chat.

To me, app/src is the better naming convention. That seems to be the convention in a lot of JS projects. It also avoid confusion if there is any compilation going on.

mserranom commented 5 years ago

ok, will move to /src

40thieves commented 5 years ago

Could we open an issue to move chat to src too?

mserranom commented 5 years ago

The build scripts are incompatible with /src in tests: https://github.com/overleaf/dev-environment/blob/master/build_scripts/v1.1.21/package.scripts.es.json#L8

mserranom commented 5 years ago

and also incompatible with nodemon from build scripts: https://github.com/overleaf/dev-environment/blob/master/build_scripts/v1.1.21/nodemon.json

I think it's better to keep it as /js