-
Notifications
You must be signed in to change notification settings - Fork 144
docs: update for v3 and add migration guide #1253
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR overhauds the documentation for the v3 release and implements a significant breaking change by renaming the abstract Matcher
and Generator
classes to AbstractMatcher
and AbstractGenerator
respectively.
- Comprehensive migration guide added for v2 to v3 transition
- Consumer and provider documentation rewritten with new v3 interface examples
- Documentation improvements including expanded matching examples and generator usage
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/pact/match/matcher.py |
Renames Matcher to AbstractMatcher and updates all type annotations and references |
src/pact/generate/generator.py |
Renames Generator to AbstractGenerator and updates docstrings |
src/pact/match/__init__.py |
Updates imports and adds comprehensive module documentation with examples |
src/pact/generate/__init__.py |
Updates imports and adds detailed generator documentation |
src/pact/interaction/_*.py |
Updates type annotations to use AbstractMatcher |
docs/consumer.md |
Complete rewrite with v3 API examples and modern patterns |
docs/provider.md |
Complete rewrite with new Verifier class usage |
MIGRATION.md |
New comprehensive migration guide for v2 to v3 transition |
Other files | Minor documentation updates and configuration changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
474f3c6
to
24f8bfd
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1253 +/- ##
====================================
- Coverage 52% 52% -1%
====================================
Files 32 32
Lines 3730 3740 +10
====================================
Hits 1967 1967
- Misses 1763 1773 +10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ceeef32
to
7ee79f6
Compare
|
||
<!-- markdownlint-disable code-block-style --> | ||
|
||
=== "Local Files" |
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.
can this be a url , for verifications by webhook?
how do we add auth in that case?
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.
this is the last clarification I really want to mop up before we box this off. If we don't have this functionality, then we can just ticket it up and work forward. If the func does exist, it would be great to document, as to reach pact nirvana, you have two provider verification flows, one by url (triggered by webhook) and one by querying the pact broker with selectors which returns dynamic results.
This was a distinct gap in V2
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.
Good shout Yousaf, we can close that off with this release too!
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.
I have no tested the webhook flow at all... so I'll take a look and see.
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.
added comment about msg pact, forgot to submit review as had to go into few meetings
Thanks @YOU54F for the thorough feedback! I've incorporated your changes and have updated the migration guide. As for the message-related docs, I want to address that in a separate PR as that is entirely new for V3. |
7ee79f6
to
d9b338c
Compare
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.
I only have a few comments to review, it's a great guide (and we can always continue to improve it once people start taking it for a spin).
|
||
<!-- markdownlint-disable code-block-style --> | ||
|
||
=== "Local Files" |
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.
Good shout Yousaf, we can close that off with this release too!
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.
last round of comments, nothing too big
/path/to/pacts/consumer-provider.json \ | ||
--consumer-app-version 1.0.0 \ | ||
--tag-with-git-branch \ | ||
--auto-detect-version-properties |
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.
lets avoid tags if we can, and remove the consumer-app-version, as its preferred there is at least a sha identifier as part of the version, which auto-detect-version-properties
will pick up
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.
So remove the --tag-with-git-branch
option?
Update: I've removed it from the latest update to this PR.
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.
thank you, I'd like to remove consumer-app-version
if possible too, or maybe update to be the <python project version>-<git sha>
if you want to retain it.
d9b338c
to
e450fbb
Compare
I've pushed the latest changes:
I do need to check the UPDATE: last commit should clarify this. |
Make clear it is abstract by naming it `AbstractMatcher`. BREAKING CHANGE: The abstract `pact.match.Matcher` class has been renamed to `pact.match.AbstractMatcher`. Signed-off-by: JP-Ellis <[email protected]>
Signed-off-by: JP-Ellis <[email protected]>
Signed-off-by: JP-Ellis <[email protected]>
Make clear it is abstract by naming it `AbstractGenerator`. BREAKING CHANGE: The abstract `pact.generate.Generator` class has been renamed to `pact.generate.AbstractGenerator`. Signed-off-by: JP-Ellis <[email protected]>
6e7366f
to
32a42b1
Compare
32a42b1
to
03b82ba
Compare
Signed-off-by: JP-Ellis <[email protected]>
Allows the broker source information to be populated from environment variables. This still requires the `broker_source` method to be called: ```python ( Verifier("my-provider") # ... .broker_source() ) ``` This can be disabled by setting the new `use_env` argument to `False` (defaults to True), or by setting the relevant argument to `None` if you want to disable an input. Signed-off-by: JP-Ellis <[email protected]>
Signed-off-by: JP-Ellis <[email protected]>
03b82ba
to
b64a4c4
Compare
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.
Looking great now, and we can add a full messages docs in a follow up PR and also look at the webhook flow (pact by url). it isn't something present in v2, so doesn't really need consideration for migration, but definitely helps us be more pact nirvana compliant :)
Great stuff Josh
📝 Summary
Overhaul of the documentation in preparation of the V3 release.
In particular:
🚨 Breaking Changes
The abstract
Matcher
andGenerator
classes have been renaned toAbstractMatcher
andAbstractGenerator
. This is not going to be a breaking change unless you have created your own matcher or generator.🔥 Motivation
To help with the adoption of the soon-to-be-released version 3, for both new and existing Pact Python users.
🔨 Test Plan
🔗 Related issues/PRs
v3
of Pact Python #1222