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

Add helper to go from []T to []*T for string, int64, float64, bool #837

Merged

Conversation

sobel-stripe
Copy link
Contributor

I ran into a common mistake in golang when writing my own code to go from a []string to []*string for use in the PaymentMethodTypes field.

This adds a helper function to do it correctly. If we want to merge this PR, I also think that we should update the examples in the documentation to use it.

@remi-stripe
Copy link
Contributor

r? @brandur-stripe Do you think we should add this? If so we would need to also update all tests to use this instead of the current approach.

@brandur-stripe
Copy link
Contributor

Yeah, this makes sense to me — the AWS SDK has it already, and it'll make a lot of our test code cleaner as it becomes possible to leave out all the individual elements' own stripe.String.

@sobel-stripe Thanks! Would you mind updating this so that there's one for all the types that we support? (e.g. Int64Slice) It shouldn't just be a string-specific construct.

@sobel-stripe sobel-stripe changed the title Add helper to go from []string to []*string Add helper to go from []T to []*T for string, int64, float64, bool Apr 17, 2019
@sobel-stripe
Copy link
Contributor Author

@brandur-stripe done, and updated the test call sites [could only find string] as well

@brandur-stripe
Copy link
Contributor

Thanks @sobel-stripe (and wow, thanks a lot for updating all those other tests).

Sorry to be a pain, but one more small-ish ask: would you mind putting tests in stripe_test.go for all the different new functions for non-string types as well? I know the type system pretty much guarantees that we're good here, but just seems good for general hygiene.

ptal @sobel-stripe

@sobel-stripe
Copy link
Contributor Author

@brandur-stripe no problem — added the tests

@brandur-stripe
Copy link
Contributor

Thanks @sobel-stripe! LGTM.

@brandur-stripe brandur-stripe merged commit 69f3f66 into stripe:master Apr 18, 2019
@brandur-stripe
Copy link
Contributor

Released as 60.6.0.

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

Successfully merging this pull request may close these issues.

3 participants