-
Notifications
You must be signed in to change notification settings - Fork 156
Currency misspelled in graphql attributes #465
Currency misspelled in graphql attributes #465
Conversation
Here @naydav requested to keep both values because of BIC (Backward Incompatible Change). Read more about BIC because this could be also requested here. But don't push any commits while not requested. It's just information, not request. backward-incompatible-changes |
@@ -10,8 +10,8 @@ type Query { | |||
type Currency { | |||
base_currency_code: String | |||
base_currency_symbol: String | |||
default_display_currecy_code: String |
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.
It will Backward Incompatible Changes
We need to mark old fields as deprecated
instead of removing them
thanks
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.
Fields were marked as deprecated
@@ -10,8 +10,10 @@ type Query { | |||
type Currency { | |||
base_currency_code: String | |||
base_currency_symbol: String | |||
default_display_currecy_code: String | |||
default_display_currecy_symbol: String | |||
default_display_currecy_code: String @deprecated(reason: "Symbol was missed. Use `default_display_currency_code`.") |
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.
Looks like we can use names like code
, default_code
etc
It's part of a single object so we don't need to duplicate object name in object fields
BUT old fields should not be removed, just marked as deprecated
Hi @XxXgeoXxX, thank you for your contribution! |
Description (*)
Issue #464
Contribution checklist (*)