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.

Sign up for GitHub

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

Jump to bottom

feat: Enhance provider states for pact-message #322

Merged
merged 2 commits into from Jan 24, 2023
Merged

feat: Enhance provider states for pact-message #322

merged 2 commits into from Jan 24, 2023

Conversation

nsfrias
Copy link
Contributor

@nsfrias nsfrias commented Nov 26, 2022

  • feat: allow multiple provider states to be defined in a pact

  • feat: allow additional parameters to be passed in the provider state

Signed-off-by: Nuno Frias nsfrias@gmail.com

feat: Enhance provider states for pact-message
0971c24 
* feat: allow multiple provider states to be defined in a pact

* feat: allow additional parameters to be passed in the provider state

Signed-off-by: Nuno Frias <nsfrias@gmail.com>
@nsfrias nsfrias mentioned this pull request Nov 26, 2022
Closed
@mefellows
Copy link
Member

Thank you! I'm surprised this works, because Ruby doesn't support V3 contracts (properly) but I'm assuming it does work otherwise you wouldn't have submitted the PR.

I think it's OK to support this, assuming the code quality/review passes.

@nsfrias
Copy link
Contributor Author

nsfrias commented Dec 6, 2022

The Ruby core is surprisingly flexible when it comes to the content of the providerStates section. I did test the full flow i.e. create pact, publish pact and verify pact, so I am confident the change works.

mikegeeves
mikegeeves reviewed Jan 10, 2023
View reviewed changes
pact/message_pact.py Outdated Show resolved Hide resolved
@mikegeeves
Copy link
Contributor

It looks fine as far as I can see [haven't tried testing it manually though]

TIL: using <= for testing for subsets, had not seen that before thanks! :D

feat: Enhance provider states for pact-message
edd73bb 
* feat: allow multiple provider states to be defined in a pact

* feat: allow additional parameters to be passed in the provider state

Signed-off-by: Nuno Frias <nsfrias@gmail.com>
@nsfrias nsfrias force-pushed the feat/multiple-provider-states-for-pact-message branch from aefd5ef to edd73bb Compare January 13, 2023 14:18
@nsfrias
Copy link
Contributor Author

nsfrias commented Jan 13, 2023

@mikegeeves All comments addressed. Btw the <= was copied from another existent test. It was new to me too.

@nsfrias
Copy link
Contributor Author

nsfrias commented Jan 18, 2023

@mikegeeves is it possible to merge, assuming you're happy with my fixes.

mikegeeves
mikegeeves approved these changes Jan 22, 2023
View reviewed changes
Copy link
Contributor

@mikegeeves mikegeeves left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me :)

elliottmurray
elliottmurray approved these changes Jan 24, 2023
View reviewed changes
Copy link
Contributor

@elliottmurray elliottmurray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on Mikes comments

@elliottmurray elliottmurray merged commit 1bbdd37 into pact-foundation:master Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elliottmurray elliottmurray elliottmurray approved these changes

@mikegeeves mikegeeves mikegeeves approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nsfrias @mefellows @mikegeeves @elliottmurray