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

Set Request.GetBody #711

Merged
merged 2 commits into from
Oct 29, 2018
Merged

Set Request.GetBody #711

merged 2 commits into from
Oct 29, 2018

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Oct 29, 2018

Here we set Request.GetBody, which is used under certain circumstances
by the internal net/http core like in the case of a redirect. This
will usually not end up being used, but it doesn't hurt to set it in
case it is (it's run lazily, and few additional resources are needed in
case it's not run).

Fixes #710.

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

Here we set `Request.GetBody`, which is used under certain circumstances
by the internal `net/http` core like in the case of a redirect. This
will usually not end up being used, but it doesn't hurt to set it in
case it is (it's run lazily, and few additional resources are needed in
case it's not run).

Fixes #710.
@remi-stripe
Copy link
Contributor

@brandur-stripe looks like this does not work on Go 1.7

@brandur-stripe
Copy link
Contributor

brandur-stripe commented Oct 29, 2018

@remi-stripe Doh! Yeah, I just saw that.

Any FUD for dropping support for <= 1.7? It's not officially supported by the Go core team, and hasn't been since the release of Go 1.9 in August 2017 (1+ year) according to their version support policy.

Also, looking at OB's language metrics page, <= 1.7 is roughly ~= 1.5% of our users, which is quite small. Those users will also likely be able to upgrade easily because there are ~no breaking changes on the path to a more modern Go version.

@remi-stripe
Copy link
Contributor

@brandur-stripe Based on your stats that sounds fine to me (though it becomes a major release then)

@brandur-stripe
Copy link
Contributor

Thanks Remi. Alright, I just added 1.11 to the matrix and dropped 1.7. I'll release as a major.

@brandur-stripe brandur-stripe merged commit ddcffea into master Oct 29, 2018
@brandur-stripe
Copy link
Contributor

Released as 52.0.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.

4 participants