Skip to content
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

Use type "Int" instead of "Number" for --max-warnings flag #4660

Closed
gkz opened this Issue Dec 10, 2015 · 10 comments

Comments

Projects
None yet
5 participants
@gkz
Copy link
Contributor

gkz commented Dec 10, 2015

Just something I noticed while working around the options recently.

The max-warnings flag has the type Number, which can be any floating point value (eg. 2.3). I think a better type for the flag would be Int, which would print an error if supplied with a non-integer value.

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Dec 10, 2015

@gkz Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Dec 10, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

gyandeeps commented Dec 10, 2015

I think based on the explanation we should make this change.

@ilyavolodin ilyavolodin added accepted and removed triage labels Dec 10, 2015

@gyandeeps

This comment has been minimized.

Copy link
Member

gyandeeps commented Dec 10, 2015

I will work on this.

@gkz I tried it with Int and when I pass in --max-warnings 10.2 it gives me 10 and not throw an error. Is that what you were expecting too?

@gyandeeps gyandeeps self-assigned this Dec 10, 2015

gyandeeps added a commit that referenced this issue Dec 10, 2015

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Dec 10, 2015

While exploring this I found a small bug in levn which parses the input. On type Int levn uses parseInt, which will return 10 when given 10.2, and then type checks the input (and thus will always pass). I was planning on fixing that before submitting a pull request to change the type of max-warnings to Int.

@gyandeeps

This comment has been minimized.

Copy link
Member

gyandeeps commented Dec 10, 2015

feel free to send a PR and I can close mine then. How does that sound?

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Dec 10, 2015

OK

@gyandeeps gyandeeps removed their assignment Dec 14, 2015

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 15, 2015

@gkz @gyandeeps where are we on this?

@gyandeeps

This comment has been minimized.

Copy link
Member

gyandeeps commented Dec 15, 2015

I had the PR (#4663) but then found a bug. So @gkz decided that he will fix that bug on his end and then make a PR here.

@gkz

This comment has been minimized.

Copy link
Contributor Author

gkz commented Dec 15, 2015

I will be dealing with this in one week - my last exam is on the 21st. I don't know if I'd label this issue a bug, it's more of a user interface issue. eg. I doubt anyone is entering 10.2 instead of 10 now, but if they were, using 10.2 would still get the user the desired results.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 15, 2015

Okay thanks

@gyandeeps gyandeeps closed this in 315f272 Dec 29, 2015

gyandeeps added a commit that referenced this issue Dec 29, 2015

Merge pull request #4827 from gkz/issue4660
Fix: Change max-warnings type to Int (fixes #4660)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.