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

Allow body to be empty #16

Merged
merged 2 commits into from
Sep 13, 2016
Merged

Allow body to be empty #16

merged 2 commits into from
Sep 13, 2016

Conversation

Thomasdezeeuw
Copy link
Contributor

@Thomasdezeeuw Thomasdezeeuw commented Sep 1, 2016

This change allows the body to be completely empty, that means without
gzip headers. This a requirement with status codes that do not allow
for a body, for example 204 No Content.

It also changes to a lazily initialise method for the gzip writer.

Fixes #5.

This change allows the body to be completely empty, that means without
a gzip headers. This a requirement with status codes that do not allow
for a body, for example 204 No Content.

It also changes to a lazily initialise method for the gzip writer.

Fixes #5.
@Thomasdezeeuw
Copy link
Contributor Author

I've updated the code to run on < Go 1.7.

@Thomasdezeeuw
Copy link
Contributor Author

Ping.

@jprobinson
Copy link
Contributor

Sorry for the delay, we had a change of repo ownership in the past week and this fell through the gaps.

This looks good to me. Thanks for contributing!

@jprobinson jprobinson merged commit 44668d7 into nytimes:master Sep 13, 2016
@adammck
Copy link
Contributor

adammck commented Sep 13, 2016

Thanks for adopting this one, @jprobinson! (And sorry, @Thomasdezeeuw, for leaving you hanging.)

jprobinson added a commit that referenced this pull request Sep 13, 2016
Adding fixes to a bug introduced with PR #16
@jprobinson
Copy link
Contributor

Just to add an update...we found this introduced a bug for any handler calling WriteHeader before writing the actual body to the response.

We ended up altering the library in #19 to still allow empty responses, but only for 204 and 304.

@Thomasdezeeuw
Copy link
Contributor Author

Thanks.

@Thomasdezeeuw Thomasdezeeuw deleted the no-body branch September 13, 2016 23:20
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.

None yet

3 participants