-
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
PHPDoc fixes, CreditNote.previewLines, and autogen prep #837
Conversation
add478d
to
ca12071
Compare
a1540c2
to
215a897
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.
Amazing work, this is so exciting! I left a lot of nitpicky comments and some real blockers.
Also flaggign that I think we have the same problem as with other codegen-ed resources: we never touch the nested resources like CustomerBalanceTransaction
and TaxId
. I could be wrong, but if I'm correct, if you removed all resources files, then re-codegen you should see all the missing files. It's okay-ish if so for now and something we should fix across all codegen soon
lib/Account.php
Outdated
@@ -305,6 +358,7 @@ public function persons($params = null, $opts = null) | |||
return $obj; | |||
} | |||
|
|||
|
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.
extra new line
} | ||
} | ||
return $updateArr; | ||
} |
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.
if we could easily put the override like that at the end of the file it'd be great
@@ -30,6 +30,7 @@ class ApplicationFee extends ApiResource | |||
use ApiOperations\NestedResource; | |||
use ApiOperations\Retrieve; | |||
|
|||
|
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.
extra new line (won't flag others)
lib/SetupIntent.php
Outdated
@@ -13,14 +13,14 @@ | |||
* @property int $created | |||
* @property string|null $customer | |||
* @property string|null $description | |||
* @property mixed|null $last_setup_error | |||
* @property \Stripe\StripeObject|null $last_setup_error |
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.
Technically this is an Error
object today not sure if we care
I think that's right. Brian and Josh from Slack saw that issue when they were starting to generate |
308f6c1
to
47ef041
Compare
47ef041
to
31ff900
Compare
1555861
to
5dbc6b1
Compare
9ac8a52
to
cb5ba60
Compare
cb5ba60
to
2626423
Compare
Rebased on latest master and regenerated. I think the remaining diff is as small as it's going to get. Changes are:
|
r? @cjavilla-stripe @remi-stripe @richardm-stripe Just in case I missed anything, but I think we're ready to make the switch to codegen! |
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.
OMG it's happening.
lgtm |
I think we're good to go. Hard to see the blind spots but excited to see this. Awesome work @richardm-stripe, @ob-stripe, and @remi-stripe!! |
Released as 7.23.0. |
r? @ob
cc @stripe/api-libraries @remi-stripe @cjavilla-stripe @marko-stripe
Changes:
Session
,CreditNote
,Invoice
,InvoiceItem
,Subscription
, andSubscriptionSchedule
.SubscriptionSchedule
that were removed in the latest API versions.