-
Notifications
You must be signed in to change notification settings - Fork 7
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
rfc: Define matching rules for form-urlencoded body #99
rfc: Define matching rules for form-urlencoded body #99
Conversation
0fe042c
to
a7b122c
Compare
a7b122c
to
1d177f2
Compare
Amazing to suggest an RFC! I haven't had a chance to read over it yet, but I just want to bring attention some of the pitfalls of
This means that we cannot assume the data can be deserialised to a dictionary, and we would have to assume at the very least the data is deserialized to a The spec also implies that we only can ever have strings. We need to handle pathological cases like:
Anyway, I'll take a better look at it in the next few days :) |
👋🏾 Hey @tienvx, will take a read next week as I am stacked out this week, but thank you for raising and supporting the RFC process 👍🏾 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly, I want to reiterate that this is a great PR!
There are a few things I'm highlighting in the above comments, and I am quite keen to hear what you have to say in response 👍
Amazing work Tien incorporating my suggestions! I just have one comment in a new section you revised/added, but it is relatively minor 👍 (and sorry I didn't review this earlier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me.
Good to see that order is important, that has caught me when testing out the pact_broker-client v2 pacts created by pact-ruby against the rust based verifier, as the query order is not preserved, which causes the broker to process it as a different query. Definitely an aside for this RFC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
thanks for your hard work @tienvx looking forward to seeing this implemented in the core and available to end users |
chore: Update RFC #99 file name and metadata
No description provided.