Update standard to remove eslint-plugin-node #4146
Conversation
Overall package sizeSelf size: 6.24 MB Dependency sizes
🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4146 +/- ##
=======================================
Coverage 85.06% 85.06%
=======================================
Files 247 247
Lines 10953 10953
Branches 33 33
=======================================
Hits 9317 9317
Misses 1636 1636 ☔ View full report in Codecov by Sentry. |
BenchmarksBenchmark execution time: 2024-03-20 18:25:39 Comparing candidate commit 448500a in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 258 metrics, 7 unstable metrics. scenario:plugin-graphql-with-depth-off-18
|
Qard
force-pushed
the
update-standard
branch
4 times, most recently
from
05539da to
978e7f5
Compare
|
Just a thought, but wouldn't it be a much smaller change to adjust the rules to be as they were before instead of changing 306 files? Not against the change per se, but it's very likely to cause conflicts with every open branch, and the goal of the PR is to fix a vulnerability and not to change our rules. |
|
The whole point of using standard is to not debate style issues and just adopt what it says, so if we go and carve out a bunch of exceptions that seems like it's opening up debate about which rules we should or should not follow. Personally I felt it was better to just make the changes to adapt, though I did go for just using a bunch of per-line rule exceptions in a bunch of places just to keep the delta relatively minimal. If we would prefer to just use standard as a base and build our own ruleset on top of that then we should probably have a discussion here about which of the new rules should be kept and which should be changed. Thoughts? |
|
Apparently I completely missed that #4032 already existed. We could just land that if you prefer how it did things. 🤔 |
I don't have a strong opinion. It just felt a bit much to change 300+ files to solve a vulnerability and I would generally prefer this type of change to be as focused as possible. But at the same time, as you said updating might mean additional rules which we would want in the long run anyway. Not blocking either way, it was just an observation. |
Qard
force-pushed
the
update-standard
branch
2 times, most recently
from
2c5fcac to
54bd199
Compare
What does this PR do?
Update to latest version of
eslint-config-standardso we can eliminateeslint-plugin-node.Motivation
eslint-plugin-nodetriggers vuln scanner warnings so we should get it out of our dependency tree.Security
Datadog employees:
@DataDog/security-design-and-guidance.