NotificationsYou must be signed in to change notification settings
Fork20.6k
Star59.5k

Core: Deprecate jQuery.trim#4461

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

mgol merged 7 commits intojquery:masterfromShashankaNataraj:4363-deprecate-trim

Aug 22, 2019

Merged

Core: Deprecate jQuery.trim#4461

mgol merged 7 commits intojquery:masterfromShashankaNataraj:4363-deprecate-trim

Aug 22, 2019

Conversation

Copy link

Contributor

ShashankaNataraj commentedAug 19, 2019•
edited
Loading

Summary

Component: Core

MovedjQuery.trim() from thecore.js file into thedeprecated.js file
Moved corresponding unit test fromtest/unit/core.js totest/unit/deprecated.js

Fixes#4363

Checklist

All authors have signed the CLA athttps://cla.js.foundation/jquery/jquery
New tests have been added to show the fix or feature works
Grunt build and unit tests pass locally with these changes
If needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com

Copy link

jsf-clabot commentedAug 19, 2019•
edited
Loading

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign ourContributor License Agreement before we can accept your contribution.
1 out of2 committers have signed the CLA.

✅ ShashankaNataraj
❌ Shashanka N

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, pleaseadd the email address used for this commit to your account.

Copy link

Member

mgol commentedAug 19, 2019

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.

mgol added Core Needs review labels

Aug 19, 2019

Copy link

Member

timmywil commentedAug 19, 2019

The code changes look good.

Copy link

Member

mgol commentedAug 19, 2019

@ShashankaNataraj Please sign our CLA.

Copy link

Member

timmywil commentedAug 19, 2019

Looks like you did sign the CLA, but one of the commits has different author info. You can fix this withgit commit --amend --author=ShashankaNataraj

ShashankaNataraj added4 commits

August 19, 2019 22:10

Fixesjquery#4363

38f078e

Fixesjquery#4363

cdcb2ef

Fixesjquery#4363

03fce6a

Fixesjquery#4363

d3c1bcc

ShashankaNataraj force-pushed the4363-deprecate-trim branch from23f8e15 tod3c1bccCompare

August 19, 2019 16:41

Copy link

ContributorAuthor

ShashankaNataraj commentedAug 19, 2019

@mgol @timmywil Thanks for your guidance. I hope the above commits do the trick! The issue was I committed with my official mail ID in the git config.

The CLA was picking up my old email ID as I have contributed to jQuery before as well!

mgol requested changes

Aug 19, 2019

View reviewed changes

Copy link

Member

mgol left a comment

Choose a reason for hiding this comment

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

Please remove thetrim test fromtest/unit/basic.js, the tests there should work with deprecated APIs excluded.

Shashanka N added3 commits

August 20, 2019 09:33

Fixesjquery#4363- removed the trim test as per review comment

5fc24a3

Merge branch 'master' ofhttps://github.com/jquery/jqueryinto 4363-d…

b9f74cb

…eprecate-trim

Fixesjquery#4363- Changed assert count to get tests to pass

14954ad

Copy link

ContributorAuthor

ShashankaNataraj commentedAug 20, 2019•
edited
Loading

@mgol @timmywil Does this look correct?

mgol changed the title~~Fixes #4363~~Core: Deprecate jQuery.trim

Aug 21, 2019

mgol approved these changes

Aug 21, 2019

View reviewed changes

Copy link

Member

mgol left a 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 athttps://contribute.jquery.org/commits-and-pull-requests/#commit-guidelines. The PR title should also follow the same format as the commit title.

Copy link

Member

mgol commentedAug 22, 2019

(I updated the PR title to match the proper format)

mgol merged commit5ea5946 intojquery:master

Aug 22, 2019

mgol removed the Needs review label

Aug 22, 2019

mgol added this to the4.0.0 milestone

Aug 22, 2019

Copy link

ContributorAuthor

ShashankaNataraj commentedAug 22, 2019

@mgol Will do, thanks!

mgol added a commit that referenced this pull request

Aug 30, 2019

Deprecated: Fix AMD parameter order

f810080

Refgh-4461

GulajavaMinistudio mentioned this pull request

Aug 31, 2019

Animation Documentation ArticleGulajavaMinistudio/jquery#99

Merged

4 tasks

mgol pushed a commit that referenced this pull request

Sep 25, 2019

Core: Deprecate jQuery.trim

56e73e0

Fixesgh-4363Closesgh-4461(cherry picked from5ea5946)

mgol modified the milestones:4.0.0,3.5.0

Sep 25, 2019

Copy link

Member

mgol commentedSep 25, 2019

Moving the milestone to3.5.0 (deprecations can only happen in minor or major releases and previously we didn't plan a minor bump on the 3.x line); I cherry-picked the change (with some necessary modifications) in56e73e0.