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.

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

[codegen] Update API Resources #647

Closed
wants to merge 1 commit into from
Closed

Conversation

ob-stripe
Copy link
Contributor

r? @-stripe (feel free to change reviewer)
cc @stripe/api-libraries

  • Sanity check for correctness.
  • Ensure appropriate test coverage (ie; write tests).

@ob-stripe
Copy link
Contributor Author

@rattrayalex-stripe I opened this PR to sanity-check against the one manually written by @remi-stripe (#608) and test the codegen flow.

Issue with the generated code itself:

  • I'm not sure about this balanceTransactions() methods -- I think we should only have listBalanceTransactions(), like we only have e.g. listTaxIds.

Issues with the codegen flow:

  • the r? tag is missing the actual reviewer's name
  • would be great if the script printed a link to the newly created PR

(Closing this PR to avoid confusion since I'm going to merge the other one.)

@ob-stripe ob-stripe closed this Jun 18, 2019
@ob-stripe ob-stripe deleted the ob/codegen-dc22970 branch June 18, 2019 00:08
@rattrayalex-stripe
Copy link
Contributor

rattrayalex-stripe commented Jun 18, 2019

Ah, great, I think there was some confusion about whether things like balanceTransactions were a happy path or not. Realistically I won't get to it this week so filed https://jira.corp.stripe.com/browse/DX-4236

will fix the primary_reviewer bug in my current PR, and also add the output PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants