-
-
Notifications
You must be signed in to change notification settings - Fork 934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
infra(unicorn): numeric-separators-style #2815
Conversation
✅ Deploy Preview for fakerjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Not sure about this one. Sometimes it looks useful, at other times the numbers aren't actual numbers... |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #2815 +/- ##
=======================================
Coverage 99.95% 99.96%
=======================================
Files 2987 2987
Lines 216037 216037
Branches 946 952 +6
=======================================
+ Hits 215936 215956 +20
+ Misses 101 81 -20 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sill thinking that it looks weird for digits <= 5 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced about the need for this rule. I think we could leave it up to authors to use separators when appropriate.
I've evaluated the proposed rule again and it has become apparent for me that its implementation could generally enhance the uniformity of "magic number" definitions within a codebase. However, our repository's unique context, where numeric values often carry specific meanings, may be compromised by the adoption of numeric separator syntax. This concern was previously expressed by @matthewmayer (see here) as well. While it is possible to configure the rule to enforce separators only where they currently exist, thereby standardizing their usage (e.g., in groups of three), this approach contradicts the rationale for introducing the rule in the first place. Given these considerations, I propose that we opt not to include this rule in our ruleset. |
Team Decision
|
f3738a5
to
5bb9518
Compare
Ref: #2439
Permanently disables the
unicorn/numeric-separators-style
lint rule.