Skip to content

Conversation

@MadManMathew
Copy link

#122
let me know your'e thoughts, I know remove test cases isn't ideal but I removed the functionality where we make the value the key and the value of that key true.

@nlf
Copy link
Collaborator

nlf commented Oct 20, 2015

the removal of tests shows that this would be a breaking change. i need to think about this.

@nlf nlf added the bug label Oct 20, 2015
@nlf nlf self-assigned this Oct 20, 2015
@ljharb
Copy link
Owner

ljharb commented Feb 2, 2016

@MadManMathew If you're still interested in landing this, could you rebase (not merge) on top of latest master?

@MadManMathew
Copy link
Author

@ljharb yes will do

@MadManMathew
Copy link
Author

@ljharb I messed up the rebase, will fix.

@ljharb
Copy link
Owner

ljharb commented Feb 18, 2016

no worries, just ping me when you'd like me to take a look!

ljharb added a commit that referenced this pull request Jul 21, 2016
@ljharb
Copy link
Owner

ljharb commented Jul 21, 2016

@MadManMathew if you're still interested in landing this, please use my version of your tests (from 75fe7ce) and let's get it working without removing any existing tests. Changing them slightly may end up being fine.

ljharb added a commit that referenced this pull request Jul 22, 2016
ljharb added a commit that referenced this pull request Oct 15, 2016
@ljharb
Copy link
Owner

ljharb commented May 20, 2017

@MadManMathew if you're not interested in completing this PR, would you mind checking the "allow edits" checkbox on the right hand column?

@ljharb ljharb assigned ljharb and unassigned nlf May 20, 2017
@MadManMathew
Copy link
Author

@ljharb sorry will do

ljharb added a commit to MadManMathew/qs that referenced this pull request May 30, 2017
@ljharb
Copy link
Owner

ljharb commented May 30, 2017

I've rebased this and included my tests; but I'm not sure how to adapt your fix to make the tests pass.

@ljharb
Copy link
Owner

ljharb commented Nov 24, 2018

@MadManMathew are you interested in pursuing this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants