-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
Core: Deprecate jQuery.trim #4461
Conversation
Shashanka N seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. |
Thanks for the PR. Please follow our guidelines, especially for commit messages: https://contribute.jquery.org/commits-and-pull-requests/ The PR title should be named in the same format as well. |
The code changes look good. |
@ShashankaNataraj Please sign our CLA. |
Looks like you did sign the CLA, but one of the commits has different author info. You can fix this with |
23f8e15
to
d3c1bcc
Compare
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.
Please remove the trim
test from test/unit/basic.js
, the tests there should work with deprecated APIs excluded.
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.
Looks good, thanks!
In the future, please name your commits according to our guidelines at https://contribute.jquery.org/commits-and-pull-requests/#commit-guidelines. The PR title should also follow the same format as the commit title.
(I updated the PR title to match the proper format) |
@mgol Will do, thanks! |
Moving the milestone to |
Summary
Component: Core
jQuery.trim()
from thecore.js
file into thedeprecated.js
filetest/unit/core.js
totest/unit/deprecated.js
Fixes #4363
Checklist