-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
fix(eslint-plugin): [no-magic-numbers] add support for enums #543
fix(eslint-plugin): [no-magic-numbers] add support for enums #543
Conversation
Codecov Report
@@ Coverage Diff @@
## master #543 +/- ##
==========================================
+ Coverage 94.21% 94.22% +<.01%
==========================================
Files 105 105
Lines 4341 4345 +4
Branches 1193 1195 +2
==========================================
+ Hits 4090 4094 +4
Misses 146 146
Partials 105 105
|
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.
thanks for working on this!
please add an option to gate this functionality with a default set to off, and document it in the rule's readme
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.
awesome, thanks!
LGTM
Fixes #573.
Currently, the following code will fail to lint using the default no-magic-numbers configuration
One could make the argument that perhaps we instead want to define it in some other way, perhaps using constants and referring to them such as:
However, this will result in a compiler error
Computed values are not permitted in an enum with string valued members.ts(2553)
.I made these changes to resolve that problem. I tried to follow the existing code pattern as much as possible, however, I am open to changes. I also assumed updating the documentation would be unnecessary for this fix, let me know if we would like a change there and I'll handle it.
Thanks for your time!