search

neuroanatomy / BrainBox

BrainBox is a web application that lets you annotate and segment 3D brain imaging data in real time, collaboratively.
https://brainbox.pasteur.fr
Other
96 stars 46 forks source link

make eslint happy #168

Open katjaq opened 6 years ago

katjaq commented 6 years ago

What is the current behaviour?

Thanks to @omgitsjoao , we now have xo and can eslint our BrainBox code by running npm test. I just ran it – and it gives a greaaaaaaat number of errors you can choose from 😊

What is the expected or desired behaviour?

We would be super happy about any fixes to decrease this number little by little by little :)

daniloisr commented 6 years ago

wow, there is a lot of errors and warnings 😅 but we can start running xo --fix on the main files and later start fixing the remaining errors, can I start doing that?

theevangelista commented 6 years ago

Yea using --fix is a great start, most of the remaning issues will be about the snake_case words, they just have to be converted to camelCase, other issues that I have found running xo is a mix between the * and + operator, but I didint got into it.

daniloisr commented 6 years ago

I noticed that some files use indentation with 4 spaces and others with 2, @katjaq do you have any preference?

daniloisr commented 6 years ago

First PR open, we can review and configure XO now :D

r03ert0 commented 6 years ago

hi @daniloisr,

I tried to configure XO, and I think that I find it easier to directly configure eslint... (and the --fix switch can be quite dangerous... the automatic choice of const or let wasn't always accurate)

I added the .eslintrc file that we used for MicroDraw, and used it to lint /js/atlasMakerServer.js. Now it's down from >1200 errors to 98 \o/ !

Is it possible to configure XO using .eslintrc? (for the moment I'll stick to eslint)

daniloisr commented 6 years ago

Hi @r03ert0,

I'm also new to XO too. The difference between it and ESlint is that XO applies a set of default config to the ESlint (you can check the configs here). It turns on a lot of ESlint configs that normally are disabled by default, that's why it throws a lot of errors.

I think that it's safer to use just ESlint for now because BrainBox has a lot of code and would be hard to fix all the errors pointed by XO. XO seems to be a great option for projects that are starting from zero. In the future, when the codebase has a better quality, we can think of adding it again.

Any thoughts @omgitsjoao?

daniloisr commented 6 years ago

@r03ert0 I created a PR addressing this request https://github.com/OpenNeuroLab/BrainBox/pull/183 Feel free to change or close it if you have another plan :D

r03ert0 commented 5 years ago

Hey hacktoberfesters! eslint is still unhappy... most .js files have <20 complaints. Could you fix them?

Some are tricky... our eslintrc.js file only likes functions with <10 statements, for example... could you make our code shorter?