Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

no-magic-numbers rule ignores arguments in Number prototype methods #3668

Merged

Conversation

mateuszwitkowski
Copy link
Contributor

@mateuszwitkowski mateuszwitkowski commented Jan 20, 2018

PR checklist

Overview of change:

no-magic-numbers rule now ignores arguments in following default Number prototype methods:

toString
toFixed
toPrecision
toExponential

CHANGELOG.md entry:

[enhancement]: no-magic-numbers rule ignores arguments passed into default Number methods

Copy link
Contributor

@jkillian jkillian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me @mateuszwitkowski, thanks! Would you mind merging master into your branch so that the tests pass?

Copy link

@giladgray giladgray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mateuszwitkowski this is sweet! few small changes please.

test/rules/no-magic-numbers/custom/test.ts.lint Outdated Show resolved Hide resolved
src/rules/noMagicNumbersRule.ts Outdated Show resolved Hide resolved
@giladgray
Copy link

@mateuszwitkowski please update this branch (fix build & merge conflicts) so we can review it, or close it if no longer relevant. we will close this if we do not hear from you in two weeks.

@mateuszwitkowski
Copy link
Contributor Author

@giladgray I've just updated this PR so it's ready to go.

@JoshuaKGoldberg JoshuaKGoldberg merged commit 484d429 into palantir:master Jun 15, 2019
@JoshuaKGoldberg
Copy link
Contributor

👋 @mateuszwitkowski, just a heads up that this is merged in now and will be available in the next TSLint release. Sorry it took so long to get in! The changes look great.

@mateuszwitkowski
Copy link
Contributor Author

@JoshuaKGoldberg Great! Thanks for the notice :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants