search

PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.
GNU Affero General Public License v3.0
32 stars 86 forks source link

Added Linting Rule #1708

Open psankhe28 opened 2 weeks ago

psankhe28 commented 2 weeks ago

Description

1665

Changes

The no-console linting rule in ESLint discourages or prevents the use of console.log statements in your JavaScript codebase while allowing the use of other console methods like console.error, console.warn, console.info, and console.table.

Screenshots

I have tested it on a file called test.js and test1.js. image image

Output: image

Tests

17/17 passed

anth-volk commented 1 week ago

Thanks for this contribution @psankhe28. Before I review, I just want to make sure - does this PR also remove garbage console.log statements and convert relevant ones to console.debug, console.error, etc.?

psankhe28 commented 1 week ago

Thanks for this contribution @psankhe28. Before I review, I just want to make sure - does this PR also remove garbage console.log statements and convert relevant ones to console.debug, console.error, etc.?

It removes all console.log and allows console.debug, error etc.

anth-volk commented 1 week ago

I'm not sure you understood my question. In at least 8 files (they're actually highlighted by GitHub in the "Files changed" tab), there are still console.log statements that will be errors if we merge this code. This issue asks contributors to remove garbage statements (e.g., on line 14 in src/layout/Screenshottable.jsx) and to transfer relevant ones (e.g., line 69 in src/api/parameters.js) to either console.error or console.debug. This is also why the GitHub Actions that run now fail - there's like 20 statements that fail this added linting rule.

psankhe28 commented 1 week ago

I'm not sure you understood my question. In at least 8 files (they're actually highlighted by GitHub in the "Files changed" tab), there are still console.log statements that will be errors if we merge this code. This issue asks contributors to remove garbage statements (e.g., on line 14 in src/layout/Screenshottable.jsx) and to transfer relevant ones (e.g., line 69 in src/api/parameters.js) to either console.error or console.debug. This is also why the GitHub Actions that run now fail - there's like 20 statements that fail this added linting rule.

I think so I misunderstood the requirement. I will look into it again.

anth-volk commented 1 week ago

That's alright. The approach you took was good - we are looking to do what you did - just to also modify those existing statements.