eBay / eslint-config-ebay

MIT License
13 stars 11 forks source link

Change no-console, with exceptions #6

Closed seangates closed 6 years ago

seangates commented 6 years ago

Description Update the setting for no-console to the following:

"no-console": ["error", { allow: ["warn", "error"] }]

Reasoning There are very few reasons we should be doing a straight console.log in production. Warn and error are much less of a problem. Otherwise, this should enforce it for most cases.

Reference: https://eslint.org/docs/rules/no-console

senthilp commented 6 years ago

A lot of folks also write tools & scripts in Node.js. console is a requirement there. How do we handle those?

seangates commented 6 years ago

Since those are the exception, they should do a project-level override in the .eslintrc for that project. Otherwise, we discourage it for code that shouldn't be using console.

senthilp commented 6 years ago

@mikewoo200 @mwoo @yomed What do you think?

mikewoo200 commented 6 years ago

I am for this suggestion, especially after seeing many forgotten console.log on the browser side in recent code reviews.

If the app is organized as Node-side main folder vs. client-side main folder, using per-directory .eslintrc would take care of allowing some/all console functions for Node-side only.

https://eslint.org/docs/user-guide/configuring#configuration-cascading-and-hierarchy

seangates commented 6 years ago

Agree. It's better that bad console.log's are suppressed, rather than the complete freedom to put them in. We should be limiting exposure at the same time providing the documentation to override if necessary.

seangates commented 6 years ago

Shall I make a PR?

senthilp commented 6 years ago

Closed with #7