Renaming all properties, structs and methods to match the API naming #459

remi-stripe · 2017-09-19T23:19:14Z

This PR ensures that all properties in the library matches the official API. Names are not shortened anymore making it easier to associated a given parameter to its API counterpart.

We can't list all the changes one by one as those are extensive but here are the ones likely to impact most integrations:

All properties with the word url in it now have the same case such as BusinessURL.
All properties with the word four in it now use a digit such as DynamicLast4 or SSNLast4.
Renamed confusing properties such as Account on BankAccount becoming AccountNumber to avoid confusing with its Connect counterpart which is now named Account instead of AccountID.
Properties such as MonthAnchor or constants such as Month are now renamed to match the corresponding parameter/value in the API MonthlyAnchor and Monthly.
Meta and Live have been renamed to Metadata and Livemode on all classes
Values on List objects has been renamed to Data
Some classes have been renamed.
- Owner is now AdditionalOwner,
- Transaction is now BalanceTransaction with all constants and corresponding classes renamed to match this
- references to Tx were also replaced by BalanceTransaction in classes, properties and constants
- Fee and FeeRefund have been renamed to ApplicationFee and ApplicationFeeRefund along with all related classes, properties and constants.
- Sub has been renamed to Subscription along with all related classes, properties and constants.
Card parameters have been renamed such as City becoming AddressCity and Month becoming `ExpMonth.
Errors codes have been prefixed with ErrorCode such as ErrorCodeCardDeclined or ErrorCodeIncorrectZip.
Event properties are now named Object and PreviousAttributes and methods GetObjectValue() and GetPreviousValue().
Pagination properties have been renamed to StartingAfter, EndingBefore and HasMore.
AddMeta() has been renamed to AddMetadata(), Expand() to AddExpand()

ob-stripe · 2017-09-20T10:17:16Z

We can see it touches a lot of fields and makes the PR hard to review because of identation.

Protip: you can add ?w=1 at the end of the URL to ignore whitespace changes.

https://github.com/stripe/stripe-go/pull/459/files?w=1

brandur-stripe · 2017-10-17T17:40:10Z

Just to double-check: this one is all set for review right Remi?

remi-stripe · 2017-10-17T18:01:25Z

@brandur-stripe Not quite yet. You wanted to fix the bool representation in the library first. There are also a few pending question I've shared with you internally.

remi-stripe · 2017-10-19T16:42:12Z

@brandur-stripe I think this is ready to review. I will update the description to list all changes that we had in that library.

As discussed internally, I will separately open a PR that lets us move all bool in params structs to pointers.

remi-stripe · 2017-10-19T19:09:07Z

Okay rebased on the latest master as a few things had changed.

brandur-stripe · 2017-10-20T15:53:15Z

dispute.go

-	URL     string `json:"url"`
+	Created  int64  `json:"created"`
+	ID       string `json:"id"`
+	MimeType string `json:"mime_type"`


Should we do MIMEType? (Multipurpose Internet Mail Extensions)

brandur-stripe · 2017-10-20T15:58:05Z

invoiceitem.go

 	Period       *Period           `json:"period"`
 	Plan         *Plan             `json:"plan"`
 	Proration    bool              `json:"proration"`
 	Quantity     int64             `json:"quantity"`
-	Sub          string            `json:"subscription"`
+	Subscription string            `json:"subscription"`


Since we're about to break it anyway, should we make Subscription expandable here?

brandur-stripe · 2017-10-20T15:59:24Z

order.go

-	Paid      int64 `json:"paid"`
-	Returned  int64 `json:"returned"`
+	Canceled int64 `json:"canceled"`
+	Fulfiled int64 `json:"fulfiled"`


I think we should leave this as Fulfilled and fix it server side at some point instead.

No point in cargo culting too hard.

Hmmm I have seen confusion in other libraries such as stripe/stripe-dotnet#1085 so I think it's worth matching the API instead of trying to write words differently in code than we do in the API. wdyt?

brandur-stripe · 2017-10-20T16:00:18Z

payout.go

+	Amount                    int64               `json:"amount"`
+	ArrivalDate               int64               `json:"arrival_date"`
+	BalanceTransaction        *BalanceTransaction `json:"balance_transaction"`
+	BankAccoun                *BankAccount        `json:"bank_account"`


Typo here. Missing a "t" on the end.

brandur-stripe · 2017-10-20T16:02:52Z

subscription.go

@@ -0,0 +1,133 @@
+package stripe


I see the rational here, but I wonder if we should leave this as sub.go so that you can always match the name of a Go file with the name of the corresponding package? (sub/)

Agreed, I just renamed those back.

brandur-stripe · 2017-10-20T16:07:34Z

Great work Remi. Can't wait for this to come in!

This is definitely going to be a little painful, but even so, probably time to rip the bandaid off. I left a few comments above, but otherwise LGTM.

remi-stripe · 2018-01-07T20:56:04Z

@brandur-stripe Okay, I addressed all remaining comments I think. Can you let me know if you have any other feedback?

This one will have to be merged after we merge #507 into it but I think we can get this done this week!

brandur-stripe · 2018-01-08T21:26:51Z

payout.go

+	ArrivalDate               int64               `json:"arrival_date"`
+	Automatic                 bool                `json:"automatic"`
+	BalanceTransaction        *BalanceTransaction `json:"balance_transaction"`
+	BankAccoun                *BankAccount        `json:"bank_account"`


Misspelling: BankAccount.

ugh, nice catch, fixed.

brandur-stripe · 2018-01-08T21:27:36Z

recipienttransfer.go

+	Recipient           *Recipient                   `json:"recipient"`
+	Reversals           *ReversalList                `json:"reversals"`
+	Reversed            bool                         `json:"reversed"`
+	SourceTransation    *BalanceTransactionSource    `json:"source_transaction"`


Misspelling: SourceTransaction.

ugh, nice catch, fixed.

brandur-stripe · 2018-01-08T21:29:45Z

@remi-stripe Nice! I took one more pass; I can't guarantee I got everything, but only found some minor stuff. Otherwise, this LGTM.

remi-stripe · 2018-02-10T20:23:36Z

Rebased on the latest master. I had to squash all commits into one readable long one because there were too many merge conflicts on small ones and it was easy to miss something and introduce a regression

brandur-stripe · 2018-02-16T22:23:33Z

Rebased on the latest master. I had to squash all commits into one readable long one because there were too many merge conflicts on small ones and it was easy to miss something and introduce a regression

Nice Remi! What do you say that we try to do the work to get this merged ASAP early next week ... before you get too tired to continue this project ;)

I'm not sure if you had a particular timeline in mind, but it seems like we're going to have to pull the trigger on this at some point. We're also sitting at v29.* right now, and v30.* would be a nice round number of a big breaking change like this one :)

remi-stripe · 2018-02-16T22:58:27Z

@brandur-stripe I'd love to do that. This can't be merged as is, there's the other PR waiting where I'm blocked for now though. It's unlikely to be cleaned up by early next week though and then I'm OOO for a bit.

brandur-stripe · 2018-02-17T00:17:45Z

This can't be merged as is, there's the other PR waiting where I'm blocked for now though. It's unlikely to be cleaned up by early next week though and then I'm OOO for a bit.

Ah k :/ Well in that case let's just try to get it on the board as soon as we can.

remi-stripe · 2018-02-17T01:28:45Z

I've had my eyes on version 30.0.0 for a while though so I'll try to get this done!

kerak19 · 2018-03-28T08:27:02Z

Hello. Any updates on this PR? Is there any approximate date when it will be merged?

brandur-stripe · 2018-04-09T18:08:27Z

@kerak19 Sorry about the delayed response here. We're not exactly sure when this will come in, but Remi and I are meeting a little later today, and I'm hopeful that we can get this in within the next couple weeks.

remi-stripe · 2018-04-14T18:36:43Z

Moved to #544 as the bigger PR

* Slight custom field description improvement * Wording and organization improvement on sync preferences page

remi-stripe force-pushed the remi-rename-all-fields branch from 14eb36f to 264b7ec Compare September 19, 2017 23:47

remi-stripe mentioned this pull request Sep 28, 2017

Parse zero-val pointers #468

Merged

remi-stripe force-pushed the remi-rename-all-fields branch from 1dc074e to eeaaa19 Compare October 7, 2017 13:56

remi-stripe force-pushed the remi-rename-all-fields branch 2 times, most recently from 13f9942 to ca7110b Compare October 16, 2017 14:36

remi-stripe force-pushed the remi-rename-all-fields branch from 6d9985b to 88a3345 Compare October 17, 2017 20:15

remi-stripe changed the title ~~[WIP] Trying to rename fields to match their API names~~ Renaming all properties, structs and methods to match the API naming Oct 19, 2017

remi-stripe force-pushed the remi-rename-all-fields branch from 88a3345 to 5328d5d Compare October 19, 2017 19:08

brandur-stripe reviewed Oct 20, 2017

View reviewed changes

remi-stripe mentioned this pull request Dec 6, 2017

Create plans and products using 2017-12-01 API version format #496

Closed

remi-stripe force-pushed the remi-rename-all-fields branch from 5328d5d to 7955210 Compare December 12, 2017 15:18

remi-stripe force-pushed the remi-rename-all-fields branch from 7955210 to d788299 Compare January 7, 2018 18:29

remi-stripe added the breaking-api-change label Jan 7, 2018

remi-stripe mentioned this pull request Jan 7, 2018

Move all booleans to boolean pointers #507

Closed

brandur-stripe reviewed Jan 8, 2018

View reviewed changes

remi-stripe force-pushed the remi-rename-all-fields branch from e6af552 to b16e137 Compare February 10, 2018 20:22

remi-stripe added 2 commits February 17, 2018 08:45

Rename all properties in the library to match the API reference

424d222

Add methods to manage parameter types as pointers

324b82c

remi-stripe force-pushed the remi-rename-all-fields branch from b16e137 to 324b82c Compare February 17, 2018 13:45

remi-stripe mentioned this pull request Apr 14, 2018

Rename all properties in the library to match the API reference #544

Merged

remi-stripe closed this Apr 14, 2018

remi-stripe deleted the remi-rename-all-fields branch February 3, 2019 00:14

nadaismail-stripe pushed a commit that referenced this pull request Oct 18, 2024

Fix where clause wording in ui (#459)

e3ab4d6

* Slight custom field description improvement * Wording and organization improvement on sync preferences page

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Renaming all properties, structs and methods to match the API naming #459

Renaming all properties, structs and methods to match the API naming #459

remi-stripe commented Sep 19, 2017 •

edited

Loading

ob-stripe commented Sep 20, 2017

brandur-stripe commented Oct 17, 2017

remi-stripe commented Oct 17, 2017

remi-stripe commented Oct 19, 2017

remi-stripe commented Oct 19, 2017

brandur-stripe Oct 20, 2017 •

edited

Loading

remi-stripe Jan 7, 2018

brandur-stripe Oct 20, 2017

remi-stripe Jan 7, 2018

brandur-stripe Oct 20, 2017

remi-stripe Jan 7, 2018

brandur-stripe Oct 20, 2017

brandur-stripe Oct 20, 2017

remi-stripe Jan 7, 2018

brandur-stripe commented Oct 20, 2017

remi-stripe commented Jan 7, 2018

brandur-stripe Jan 8, 2018

remi-stripe Jan 8, 2018

brandur-stripe Jan 8, 2018

remi-stripe Jan 8, 2018

brandur-stripe commented Jan 8, 2018

remi-stripe commented Feb 10, 2018

brandur-stripe commented Feb 16, 2018

remi-stripe commented Feb 16, 2018

brandur-stripe commented Feb 17, 2018

remi-stripe commented Feb 17, 2018

kerak19 commented Mar 28, 2018

brandur-stripe commented Apr 9, 2018

remi-stripe commented Apr 14, 2018

Renaming all properties, structs and methods to match the API naming #459

Renaming all properties, structs and methods to match the API naming #459

Conversation

remi-stripe commented Sep 19, 2017 • edited Loading

ob-stripe commented Sep 20, 2017

brandur-stripe commented Oct 17, 2017

remi-stripe commented Oct 17, 2017

remi-stripe commented Oct 19, 2017

remi-stripe commented Oct 19, 2017

brandur-stripe Oct 20, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandur-stripe commented Oct 20, 2017

remi-stripe commented Jan 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandur-stripe commented Jan 8, 2018

remi-stripe commented Feb 10, 2018

brandur-stripe commented Feb 16, 2018

remi-stripe commented Feb 16, 2018

brandur-stripe commented Feb 17, 2018

remi-stripe commented Feb 17, 2018

kerak19 commented Mar 28, 2018

brandur-stripe commented Apr 9, 2018

remi-stripe commented Apr 14, 2018

remi-stripe commented Sep 19, 2017 •

edited

Loading

brandur-stripe Oct 20, 2017 •

edited

Loading