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

Added BalanceTransaction type constants #662

Merged
merged 1 commit into from
May 22, 2019
Merged

Added BalanceTransaction type constants #662

merged 1 commit into from
May 22, 2019

Conversation

nickdnk
Copy link
Contributor

@nickdnk nickdnk commented May 22, 2019

Pretty self-explanatory this one.

Cheers.

@nickdnk
Copy link
Contributor Author

nickdnk commented May 22, 2019

The only thing that's a bit odd here is that on https://stripe.com/docs/api/balance/balance_transaction#balance_transaction_object-fee_details, the tax_fee is called simply tax.

@ob-stripe Should we include a type tax in this list to be able to use these constants inside fee_details as well?

Copy link
Contributor

@ob-stripe ob-stripe left a comment

Choose a reason for hiding this comment

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

Thanks Nick!

@ob-stripe ob-stripe self-assigned this May 22, 2019
@ob-stripe ob-stripe merged commit 5973a92 into stripe:master May 22, 2019
@ob-stripe
Copy link
Contributor

Released as 6.36.0.

@nickdnk
Copy link
Contributor Author

nickdnk commented May 22, 2019

Hehe. That was fast. Did you get a chance to read my comment above?

@ob-stripe
Copy link
Contributor

Oops, sorry, I missed it. Technically this is a different attribute so I think it would be more appropriate to define a second set of constants FEE_DETAILS_TYPE_.... However this attribute is not defined as an enum in the OpenAPI spec (despite what the description implies) so I'd rather avoid adding constants for now. It probably should be an enum but I think the current backend implementation makes it difficult. I'll try to clarify with the owning team.

@nickdnk
Copy link
Contributor Author

nickdnk commented May 22, 2019

@ob-stripe I also think it would be kind of silly to add three constants where two of them already exist on the same object. Much like the overlapping review_reasons on the Review object.

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

Successfully merging this pull request may close these issues.

None yet

2 participants