tediousjs / tedious

Node TDS module for connecting to SQL Server databases.
http://tediousjs.github.io/tedious/
MIT License
1.57k stars 436 forks source link

chore: Convert ES Lint to Flat Config #1631

Closed elliot-huffman closed 2 months ago

elliot-huffman commented 4 months ago

Changes:

elliot-huffman commented 4 months ago

Once the #1630 PR is merged, then I can add the IDE configs required for the new flat files into this PR

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 78.45%. Comparing base (934e98e) to head (e07eb2f).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1631 +/- ## ========================================== - Coverage 79.19% 78.45% -0.75% ========================================== Files 93 93 Lines 4860 4860 Branches 933 933 ========================================== - Hits 3849 3813 -36 - Misses 707 750 +43 + Partials 304 297 -7 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

arthurschreiber commented 4 months ago

This is great, thank you! ❤️

Would you mind changing this pull request to only focus on switching to the flat config, without changing the rules that are applied? I'm open to changing the linting rules, but I'd prefer a separate PR per changed lint rule so that we can have all config change plus all the required code changes per rule change together.

Otherwise, this PR will just become too unwieldy and impossible to review. 🙇‍♂️

elliot-huffman commented 4 months ago

Works for me, changes incoming on Monday!

Get Outlook for Androidhttps://aka.ms/AAb9ysg


From: Arthur Schreiber @.> Sent: Saturday, May 4, 2024 12:16:17 AM To: tediousjs/tedious @.> Cc: Elliot Huffman @.>; Author @.> Subject: Re: [tediousjs/tedious] chore: Convert ES Lint to Flat Config (PR #1631)

This is great, thank you! ❤️

Would you mind changing this pull request to only focus on switching to the flat config, without changing the rules that are applied? I'm open to changing the linting rules, but I'd prefer a separate PR per changed lint rule so that we can have all config change plus all the required code changes per rule change together.

Otherwise, this PR will just become too unwieldy and impossible to review. 🙇‍♂️

— Reply to this email directly, view it on GitHubhttps://github.com/tediousjs/tedious/pull/1631#issuecomment-2094000731, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABGMEWTZLYQYE5QTKZZDMNTZAROJDAVCNFSM6AAAAABHGCQOQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOJUGAYDANZTGE. You are receiving this because you authored the thread.Message ID: @.***>

elliot-huffman commented 4 months ago

@arthurschreiber, the linting rulesets have been disabled. I will enable one ruleset at a time and have the contents of the project fixed and make that a PR.

elliot-huffman commented 4 months ago

no-loss-of-precision is a new rule that was just introduced to check for number casting. It was not previously listed in the rule config and has found a bunch of potential cast errors (that look accurate to me). This commit will fail linting because of this new rule that is enabled by default. This is because the package version were bumped to 7.8.0 up from 7.0.2. 7.1.0 introduced this new rule.

arthurschreiber commented 4 months ago

The reason for the new lint failures for files in the test folder is because there exists a separate test/.eslintrc file that defines the linter configuration for these files. Can you convert that into the new config format as well? That should remove all the test failures.

We can later focus on aligning the configuration between the test folder and the rest of the source to use the same linter settings.

elliot-huffman commented 4 months ago

The reason for the new lint failures for files in the test folder is because there exists a separate test/.eslintrc file that defines the linter configuration for these files. Can you convert that into the new config format as well? That should remove all the test failures.

We can later focus on aligning the configuration between the test folder and the rest of the source to use the same linter settings.

Oh, I missed this, can do!