Skip to content

Tests: Account for array-like objects in grep() #2605

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.

Sign up for GitHub

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
Closed
AurelioDeRosa wants to merge 4 commits into from
Closed

Tests: Account for array-like objects in grep() #2605

AurelioDeRosa wants to merge 4 commits into from

Conversation

@AurelioDeRosa
Copy link
Member

@AurelioDeRosa AurelioDeRosa commented Sep 20, 2015

@AurelioDeRosa AurelioDeRosa mentioned this pull request Sep 20, 2015
Closed
@arthurvr
Copy link
Member

arthurvr commented Oct 7, 2015

Can anyone take a look at this PR? It seems pretty straightforward :)

@mgol
Copy link
Member

mgol commented Oct 7, 2015

LGTM.

@mgol mgol self-assigned this Oct 7, 2015
@mgol mgol added this to the 3.0.0 milestone Oct 7, 2015
@mgol mgol added Core Tests and removed Core labels Oct 7, 2015
@markelog
Copy link
Member

markelog commented Oct 8, 2015

Would be cool to move it to different test

@markelog
Copy link
Member

markelog commented Oct 12, 2015

Friendly ping

@AurelioDeRosa
Copy link
Member Author

AurelioDeRosa commented Oct 12, 2015

Hi @markelog.

I'll do it tonight.

@AurelioDeRosa
Copy link
Member Author

AurelioDeRosa commented Oct 12, 2015

PR updated.

@mgol
Copy link
Member

mgol commented Oct 12, 2015

@AurelioDeRosa The CLA check has failed. Perhaps you have incorrect data in one of the commits?

@AurelioDeRosa
Update core.js
b44e612 
split into two tests
@AurelioDeRosa
Copy link
Member Author

AurelioDeRosa commented Oct 12, 2015

Fixed. Sorry about that.

mgol
mgol reviewed Oct 12, 2015
View reviewed changes
test/unit/core.js Outdated
Copy link
Member

@mgol mgol Oct 12, 2015

Choose a reason for hiding this comment

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

These tests are now duplicated; they shouldn't be here at all.

markelog
markelog reviewed Oct 12, 2015
View reviewed changes
test/unit/core.js Outdated
Copy link
Member

@markelog markelog Oct 12, 2015

Choose a reason for hiding this comment

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

Could make that less then 120 symbols on the line?

Copy link
Member

@markelog markelog Oct 12, 2015

Choose a reason for hiding this comment

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

Sorry, for being so nitpicky btw

Copy link
Member Author

@AurelioDeRosa AurelioDeRosa Oct 13, 2015

Choose a reason for hiding this comment

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

@markelog Done. Please, let me know if that's OK. Also, you don't have to apologize. It's totally fine to do more work on the PR to improve its quality standard.

markelog pushed a commit that referenced this pull request Oct 13, 2015
@AurelioDeRosa @markelog
Tests: Account for array-like objects in jQuery.grep
6e466af 
Cherry-picked from 67b76f5
Closes gh-2605
@markelog markelog closed this in 67b76f5 Oct 13, 2015
@markelog
Copy link
Member

markelog commented Oct 13, 2015

Landed! Thank you

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@mgol mgol

Labels

Tests

Milestone

3.0.0

Development

Successfully merging this pull request may close these issues.

5 participants

@AurelioDeRosa @arthurvr @mgol @markelog @jquerybot