-
Notifications
You must be signed in to change notification settings - Fork 850
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
Add support for the Reporting resources #514
Conversation
12c85cc
to
6456fb8
Compare
We resolved all the issues internally and it's ready to be merged. |
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.
Left a small comment, but LGTM.
ptal @remi-stripe
lib/Reporting/ReportType.php
Outdated
* @property string $version | ||
* @property mixed $result | ||
* @property string $status | ||
* @property int $succeeded_at |
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'm not seeing result
/status
/succeeded_at
in the implementation. Are you sure those are still relevant?
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.
hmmm looks like a bad copy paste, fixing
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.
Fixed
6456fb8
to
cf8be01
Compare
@brandur-stripe PTAL (And thank you for catching this mistake) |
LGTM. |
Released as 6.17.0. |
Adding support for the new Reporting resources. Those are in OpenAPI but not yet public but since they are starting to be used and are stable we should merge them.
It's WIP for now while I get some feedback on the implementation and the shape of the resources.
cc @stripe/api-libraries
cc @brahn-stripe