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

Simplify handling of Expand and ExtraParams #1499

Merged
merged 1 commit into from
Feb 4, 2019

Conversation

ob-stripe
Copy link
Contributor

r? @remi-stripe

Simplify handling of Expand and ExtraParams in BaseOptions:

  • Expand is now annotated with [JsonProperty("expand")]
  • ExtraParams is now annotated with [JsonExtensionData]

JsonExtensionData is a special attribute in Newtonsoft.Json that instructs the JSON serializer to write the values in the collection during serialization and to deserialize values with no matching class member during deserialization.

I added a few lines of code to FormEncoder to do something similar in our own serialization process. I also had to change the type of ExtraParams from Dictionary<string, string> to Dictionary<string, object>, but I think that's better anyway, as it lets uses use non-string extra params (e.g. a list or a hash).

Also, Expand is now null by default instead of an empty list, because otherwise it would be serialized as expand= when the list is empty. Using AddExpand() will automatically create the list if necessary.

@ob-stripe ob-stripe force-pushed the ob-expand-extra-params branch from 07e121d to c841bab Compare February 2, 2019 22:46
@ob-stripe ob-stripe mentioned this pull request Feb 2, 2019
52 tasks
@remi-stripe
Copy link
Contributor

This looks great to me. There are some things I don't fully grasp so re-assign to @brandur-stripe to double check but feel free to self-approve and merge!

@ob-stripe ob-stripe force-pushed the ob-expand-extra-params branch from c841bab to 0375d37 Compare February 3, 2019 21:05
@brandur-stripe
Copy link
Contributor

Seems way cleaner to me!

LGTM.

@ob-stripe
Copy link
Contributor Author

Thanks both!

@ob-stripe ob-stripe merged commit 9d510f9 into integration-v23 Feb 4, 2019
@ob-stripe ob-stripe deleted the ob-expand-extra-params branch February 4, 2019 15:56
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.

3 participants