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

QueryMapper parses query strings including nested params #345

Merged
merged 3 commits into from Jun 22, 2014

Conversation

simonoff
Copy link
Contributor

Got the fix from #337 but refactored and more tested

@bblimke
Copy link
Owner

bblimke commented Dec 15, 2013

Thanks for that. Much more readable with extracted methods and tests :)

There are no specs for values_to_query. Does it now also work for nested hashes and arrays?

@simonoff
Copy link
Contributor Author

I'm not tested it. In my application I'm using to_param from Rails.

@simonoff
Copy link
Contributor Author

Added more test for '#to_query' method.

@bblimke
Copy link
Owner

bblimke commented Jan 1, 2014

Why have you added simplecov?

@simonoff
Copy link
Contributor Author

simonoff commented Jan 1, 2014

By simplecov i can view how many code lines covered. For example the dot notation and array notation variants not tested. And I doesn't know how they must looks like.

@simonoff
Copy link
Contributor Author

Any updates?

@bblimke
Copy link
Owner

bblimke commented Jan 23, 2014

Sorry :) One thing preventing me from merging it is tests failing on Ruby 1.8. Probably due to simplecov which doesn't support ruby 1.8.

@simonoff
Copy link
Contributor Author

Hm... You still want to support ruby 1.8 ?! It's not supported any more - https://www.ruby-lang.org/en/news/2013/06/30/we-retire-1-8-7/

@bblimke
Copy link
Owner

bblimke commented Feb 20, 2014

I don't mind supporting it in WebMock 1.x

@simonoff
Copy link
Contributor Author

@bblimke So when it will be merged? Or need to make changes first? Tell me.

@bblimke
Copy link
Owner

bblimke commented Jun 21, 2014

@simonoff I'm happy to merge it as long as simplecov is removed. It's a shame to drop 1.8 support just because of simplecov added as dependency.

@simonoff
Copy link
Contributor Author

Ok, then I will split simplecov into another PR.

@simonoff
Copy link
Contributor Author

@bblimke Done. Was synced with latest master branch.

bblimke added a commit that referenced this pull request Jun 22, 2014
QueryMapper parses query strings including nested params
@bblimke bblimke merged commit 6b5374c into bblimke:master Jun 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants