Skip to content
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

issue #122 #123

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

issue #122 #123

wants to merge 2 commits into from

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.

@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