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

Group resources with sub-resources under one folder #1309

Merged
merged 1 commit into from
Oct 1, 2018

Conversation

remi-stripe
Copy link
Contributor

Some resources go together such as Account and LegalEntity or PayoutSchedule so we group all of them under a common folder.

At the same time, I renamed a few fields on some resources to match the API (but then stopped when I realized the diff would not really show that) and added support for AddressJapan on the Account resource.

r? @ob-stripe
cc @stripe/api-libraries

@remi-stripe
Copy link
Contributor Author

One question I have is whether it'd be cleaner to have one folder per resource even if there's just one file? And if so, whether all shared/common resources are better under the resource they are closest to or under a Common folder like Address or Inventory.

@ob-stripe
Copy link
Contributor

Rename Balances/ to Balance/ to be consistent with the Services/ tree / the API.

One question I have is whether it'd be cleaner to have one folder per resource even if there's just one file?

I'd have a slight preference for always having one folder per resource, though I don't feel too strongly about it.

And if so, whether all shared/common resources are better under the resource they are closest to or under a Common folder like Address or Inventory.

I'd be in favor of using a Common folder.

@remi-stripe remi-stripe force-pushed the remi-fix-files-folders branch from 2b2a6c9 to 5eaccf8 Compare October 1, 2018 13:27
@remi-stripe
Copy link
Contributor Author

@ob-stripe Done! PTAL

@remi-stripe remi-stripe force-pushed the remi-fix-files-folders branch from 5eaccf8 to 36357c3 Compare October 1, 2018 13:32
@ob-stripe
Copy link
Contributor

Rename ThreeDSecures to ThreeDSecure. Otherwise LGTM!

Also made a few changes to some resources to match the API naming and added
support for AddressJapan on the Account resource.
@remi-stripe remi-stripe force-pushed the remi-fix-files-folders branch from 36357c3 to 31452ad Compare October 1, 2018 13:35
@remi-stripe remi-stripe merged commit 6bc3726 into integration-next-major-version Oct 1, 2018
@remi-stripe remi-stripe deleted the remi-fix-files-folders branch October 1, 2018 13:43
@ob-stripe ob-stripe mentioned this pull request Oct 1, 2018
32 tasks
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.

2 participants