Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Support multiple request_param args. #492

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

rpatterson commented Mar 15, 2012

Fixes #465, I think. :-)

I think I've got this done right, but, again, it's very naive since I don't know what any of this is for. Can someone review this pull request and educate me if I'm missing something. Then I'll add commits to address.

One of the things that may need some discussion is the difference between how the predicate __text__ was generated for request_param and match_param before my changes. match_param only had coverage for foo=bar style params, not for foo params without values and does not have coverage for multiple params. request_param had coverage for both with and without values but, obviously, had no coverage for multiple params.

match_param would format a single foo=bar param as ['foo=bar'] in __text__ while request_param would use foo = bar for a param with a value and foo for a param without a value. My fork currently supports the foo style for both match_param and request_param when there is only one param without a value. Since there was no coverage on the previous request_param foo = bar behavior for params with values but there was coverage for the match_param ['foo=bar'] behavior, I left the match_param ['foo=bar'] style for request_param as well. I've added coverage for this behavior as well. I've also added coverage for multiple params in the __text__.

Will someone review this discussion and my commits and help me understand if I'm misunderstanding things.

Owner

mcdonc commented Mar 17, 2012

I'm going to review/merge this after we release 1.3 final as it's a new feature.

Owner

mmerickel commented Aug 23, 2012

this is going to need some massaging now that @mcdonc rewrote the predicate machinery

Owner

mcdonc commented Oct 13, 2012

Superseded by pull #705

@mcdonc mcdonc closed this Oct 13, 2012

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