Skip to content

/ jquery

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

Jump to bottom

Core: Deprecate jQuery.trim #4461

Merged
merged 7 commits into from Aug 22, 2019
Merged

Core: Deprecate jQuery.trim #4461

merged 7 commits into from Aug 22, 2019

Conversation

@ShashankaNataraj
Copy link
Contributor

@ShashankaNataraj ShashankaNataraj commented Aug 19, 2019 •
edited

Summary

Component: Core

  • Moved jQuery.trim() from the core.js file into the deprecated.js file
  • Moved corresponding unit test from test/unit/core.js to test/unit/deprecated.js

Fixes #4363

Checklist
@jsf-clabot
Copy link

@jsf-clabot jsf-clabot commented Aug 19, 2019 •
edited

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 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, please add the email address used for this commit to your account.
@mgol
Copy link
Member

@mgol mgol commented Aug 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.
@timmywil
Copy link
Member

@timmywil timmywil commented Aug 19, 2019

The code changes look good.
@mgol
Copy link
Member

@mgol mgol commented Aug 19, 2019

@ShashankaNataraj Please sign our CLA.
@timmywil
Copy link
Member

@timmywil timmywil commented Aug 19, 2019

Looks like you did sign the CLA, but one of the commits has different author info. You can fix this with git commit --amend --author=ShashankaNataraj
@ShashankaNataraj
Fixes #4363
38f078e
@ShashankaNataraj
Fixes #4363
cdcb2ef
@ShashankaNataraj
Fixes #4363
03fce6a
@ShashankaNataraj
Fixes #4363
d3c1bcc
@ShashankaNataraj ShashankaNataraj force-pushed the ShashankaNataraj:4363-deprecate-trim branch from 23f8e15 to d3c1bcc Aug 19, 2019
@ShashankaNataraj
Copy link
Contributor Author

@ShashankaNataraj ShashankaNataraj commented Aug 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
mgol requested changes Aug 19, 2019
View changes
Copy link
Member

@mgol mgol left a comment

Please remove the trim test from test/unit/basic.js, the tests there should work with deprecated APIs excluded.
@ShashankaNataraj
Fixes #4363 - removed the trim test as per review comment
5fc24a3
@ShashankaNataraj
Merge branch 'master' of https://github.com/jquery/jquery into 4363-d…
b9f74cb 
…eprecate-trim
@ShashankaNataraj
Fixes #4363 - Changed assert count to get tests to pass
14954ad
@ShashankaNataraj
Copy link
Contributor Author

@ShashankaNataraj ShashankaNataraj commented Aug 20, 2019 •
edited

@mgol @timmywil Does this look correct?
@mgol mgol changed the title Fixes #4363 Core: Deprecate jQuery.trim Aug 21, 2019
@mgol
mgol approved these changes Aug 21, 2019
View changes
Copy link
Member

@mgol mgol left a comment

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.
@mgol
Copy link
Member

@mgol mgol commented Aug 22, 2019

(I updated the PR title to match the proper format)
@mgol mgol merged commit 5ea5946 into jquery:master Aug 22, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
@mgol mgol removed the Needs review label Aug 22, 2019
@mgol mgol added this to the 4.0.0 milestone Aug 22, 2019
@ShashankaNataraj
Copy link
Contributor Author

@ShashankaNataraj ShashankaNataraj commented Aug 22, 2019

@mgol Will do, thanks!
mgol added a commit that referenced this pull request Aug 30, 2019
@mgol
Deprecated: Fix AMD parameter order
f810080 
Ref gh-4461
@GulajavaMinistudio GulajavaMinistudio mentioned this pull request Aug 31, 2019
Merged
4 tasks
mgol added a commit that referenced this pull request Sep 25, 2019
@ShashankaNataraj @mgol
Core: Deprecate jQuery.trim
56e73e0 
Fixes gh-4363
Closes gh-4461

(cherry picked from 5ea5946)
@mgol mgol removed this from the 4.0.0 milestone Sep 25, 2019
@mgol mgol added this to the 3.5.0 milestone Sep 25, 2019
@mgol
Copy link
Member

@mgol mgol commented Sep 25, 2019

Moving the milestone to 3.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) in 56e73e0.
@mgol mgol mentioned this pull request Sep 26, 2019
Closed
@ShashankaNataraj ShashankaNataraj deleted the ShashankaNataraj:4363-deprecate-trim branch Oct 4, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mgol mgol

Assignees
No one assigned
Labels
Core
Projects
None yet
Milestone
3.5.0
Linked issues

Successfully merging this pull request may close these issues.

Deprecate jQuery.trim?
4 participants
@ShashankaNataraj @jsf-clabot @mgol @timmywil