1. Mailing Lists
  2. Point of Sale
  3. Re: [OCA/pos] [ADD] eslintrc file to have the possibility to mass eslint --fix JS code (#330)

Point of Sale mailing list archives

pos@odoo-community.org

Browse archives

Re: [OCA/pos] [ADD] eslintrc file to have the possibility to mass eslint --fix JS code (#330)

Version 10 return to credit card

Avatar

Re: [OCA/pos] [ADD] eslintrc file to have the possibility to mass eslint --fix JS code (#330)

by Sylvain LE GAL <notifications@github.com> - 27/02/2019 19:32:16

Hi @kirca. Thanks for your review.

does eslint catch the same errors as the current JS linter in travis?

i'm not sure. maybe @OCA/community-maintainers could answer to this question.

As far as I tested, the eslint check is more strict that the travis check. (In that direction, it's ok).

here is the extract of the eslint check. (all the things that could not be fixed automatically) :
::

20:1   error  'openerp' is not defined                                    no-undef
21:3   error  Read-only global 'module' should not be modified            no-global-assign
23:7   error  '_t' is assigned a value but never used                     no-unused-vars
44:43  error  Expected '===' and instead saw '=='                         eqeqeq
61:33  error  Identifier 'product_list' is not in camel case              camelcase
74:60  error  Typeof comparisons should be to string literals          valid-typeof
587:49  error  Unexpected newline between function name and paren        func-call-spacing
271:2   error  Mixed spaces and tabs                                     no-mixed-spaces-and-tabs
311:13  error  Unexpected tab character                                  no-tabs

Some checks are maybe a matter of taste (camelcase), but I thinks it's good to fix no-undef, no-global-assign, no-unused-vars, etc...

I'm not a lint expert and I did not benchmarked all the JS checks solutions. This is an intend to reduce the problem of huge warning logs, and provide a tools for all the contributors in OCA/pos (and also OCA/web I guess).

  • After merging, it's quite easy to ask to contributors to install eslint and to run it.
  • Also it is maybe possible to run it by bot, after each merge. (like readme generation) ? CC : @sbidoul

here is the current travis log of the branch 9.0 : https://travis-ci.org/OCA/pos/jobs/465595716
Your code has been rated at -2.77/10
(yes, negative notation seems to be possible !)

Once ran, here the new log (#331) : https://travis-ci.org/OCA/pos/jobs/499417491
Your code has been rated at 9.33/10

I see it easier to review and a good first step. Don't you think ?

regards.

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.