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

Deprecate jQuery.isNumeric #2960

Closed
markelog opened this issue Feb 29, 2016 · 15 comments
Closed

Deprecate jQuery.isNumeric #2960

markelog opened this issue Feb 29, 2016 · 15 comments
Assignees
Milestone

Comments

@markelog
Copy link
Member

@markelog markelog commented Feb 29, 2016

Was created for internal use, don't needed now

@markelog markelog added this to the 3.1.0 milestone Feb 29, 2016
@mgol
Copy link
Member

@mgol mgol commented Feb 29, 2016

👍 This API is not terribly intuitive, I'd expect people to just code sth like that themselves if they need it.

@markelog markelog added 1.x-only Core and removed 1.x-only labels Mar 3, 2016
@dmethvin
Copy link
Member

@dmethvin dmethvin commented Apr 20, 2016

I think we have a strong position for deprecating this since what people mean by "numeric" seems pretty subjective and not very useful without further validation.

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Jul 9, 2016

Can I take this up?

@markelog
Copy link
Member Author

@markelog markelog commented Jul 9, 2016

Sure, here is the link for reference - http://contribute.jquery.org/commits-and-pull-requests/

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Jul 11, 2016

@markelog Correct me if Im wrong:

  • The isNumeric function needs to be moved from core.js to deprecated.js
  • The usage within unit tests need not be changed as this function was meant to be used for unit tests from the beggining?
@timmywil
Copy link
Member

@timmywil timmywil commented Jul 11, 2016

@ShashankaNataraj I think tests can be changed later if we decide to remove isNumeric.

@mgol
Copy link
Member

@mgol mgol commented Jul 11, 2016

@timmywil I think it should be changed now, while closing this issue. We should aim at the slim build being able to pass as many tests as possible (with the exception of excluded modules) so I'd like not to go in the opposite direction. Also, that's the pattern we followed in the past, e.g. bind/unbind/delegate/undelegate tests are contained in test/unit/deprecated.js.

There are not many tests using isNumeric so it shouldn't be too hard to fix them.

@timmywil
Copy link
Member

@timmywil timmywil commented Jul 11, 2016

@mgol Yes, they should be moved, but I was talking about usage of isNumeric in the tests. We should continue testing the function itself until it's removed.

@markelog
Copy link
Member Author

@markelog markelog commented Jul 11, 2016

I think we should keep testing it, but if isNumeric used in different places it should be replaced

@mgol
Copy link
Member

@mgol mgol commented Jul 11, 2016

@timmywil I'm talking about that as well. If we just move isNumeric to deprecated.js but don't change the tests that were not testing this method but just using it then we'd introduce additional test failures when testing with the slim version.

@timmywil
Copy link
Member

@timmywil timmywil commented Jul 11, 2016

@mgol I'm proposing we replace stray uses of isNumeric that aren't directly related to testing isNumeric, move the tests that do directly test isNumeric to test/unit/deprecated (not remove them), and slim will be fine because it should not be testing against the deprecated module.

@timmywil
Copy link
Member

@timmywil timmywil commented Jul 11, 2016

@mgol By the way, the only place I can find where we use isNumeric that isn't related to testing isNumeric itself is 2 lines in test/unit/css.js.

@mgol
Copy link
Member

@mgol mgol commented Jul 11, 2016

@mgol I'm proposing we replace stray uses of isNumeric that aren't directly related to testing isNumeric, move the tests that do directly test isNumeric to test/unit/deprecated (not remove them), and slim will be fine because it should not be testing against the deprecated module.

That's what I meant as well. 😄 It seems we've had a misunderstanding.

@markelog
Copy link
Member Author

@markelog markelog commented Jul 13, 2016

Yeah, same here :)

@CrazyPython
Copy link

@CrazyPython CrazyPython commented Aug 10, 2016

@dmethvin dmethvin mentioned this issue Aug 14, 2016
5 of 8 tasks complete
@dmethvin dmethvin self-assigned this Sep 26, 2016
@timmywil timmywil modified the milestones: 3.2.0, 3.3.0 Mar 6, 2017
This was referenced Apr 2, 2017
@timmywil timmywil added the Blocker label Jun 19, 2017
jbedard added a commit to jbedard/jquery that referenced this issue Dec 13, 2017
@jbedard jbedard mentioned this issue Dec 13, 2017
4 of 4 tasks complete
jbedard added a commit to jbedard/jquery that referenced this issue Dec 13, 2017
jbedard added a commit to jbedard/jquery that referenced this issue Dec 13, 2017
jbedard added a commit to jbedard/jquery that referenced this issue Jan 5, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 8, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 9, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 9, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 9, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 13, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 13, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 14, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 15, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 15, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 15, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 15, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 16, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 16, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 16, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 16, 2018
jbedard added a commit to jbedard/jquery that referenced this issue Jan 16, 2018
jbedard added a commit that referenced this issue Jan 16, 2018
Fixes gh-2960
Closes gh-3888
@lock lock bot locked as resolved and limited conversation to collaborators Jul 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

7 participants