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

fixes error when response is null #342

Merged
merged 1 commit into from May 19, 2015
Merged

Conversation

@mgartner
Copy link
Contributor

mgartner commented May 18, 2015

This is related to #285 and #303. It looks like this part of the code was forgotten in the fix, and we are still seeing errors in 5.1.2.

@arb arb added this to the 6.1.3 milestone May 19, 2015
@arb arb self-assigned this May 19, 2015
@arb arb added the bug label May 19, 2015
arb added a commit that referenced this pull request May 19, 2015
fixes error when response is null
@arb arb merged commit 37a6279 into hapijs:master May 19, 2015
@lloydbenson

This comment has been minimized.

Copy link
Contributor

lloydbenson commented May 19, 2015

Thanks!

@mgartner mgartner deleted the mgartner:fix-null-response branch May 19, 2015
@mgartner

This comment has been minimized.

Copy link
Contributor Author

mgartner commented May 19, 2015

@lloydbenson thanks for the update. Is there anyway we could get this in a 5.1.x release? We're using good-loggly and it doesn't yet support good 6.x. continuationlabs/good-loggly#2

@arb

This comment has been minimized.

Copy link
Contributor

arb commented May 19, 2015

Feel free to back port this change as well as the other PRs listed and open a second PR against 5.1.2. We usually don't back port bugs unless there is a large number of requests for it. We like people to stay up to date 😄

@mgartner

This comment has been minimized.

Copy link
Contributor Author

mgartner commented May 19, 2015

@arb I made a quick fix here against 5.1.2: https://github.com/mgartner/good/tree/5.1.2-null-response-back-port

What branch would I create a PR onto though? There is only master.

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 19, 2015

Wouldn't it be more useful to port good-loggly to 6.x or to ask the maintainer to do this(or share his work so you can built on it or something)?

@mgartner

This comment has been minimized.

Copy link
Contributor Author

mgartner commented May 19, 2015

It looks like he's started work on it: continuationlabs/good-loggly#2

I was hoping for a more immediate solution though.

@arb

This comment has been minimized.

Copy link
Contributor

arb commented May 19, 2015

@mgartner that is only part of the update, I mean do all the changes required for this change. If you want it to work, you should apply all of the changes.

When you open a PR, you can type in "v5.2.1" as the branch.

@mgartner

This comment has been minimized.

Copy link
Contributor Author

mgartner commented May 19, 2015

What are the other changes required? Sorry, I'm not sure I'm following.

@arb

This comment has been minimized.

Copy link
Contributor

arb commented May 19, 2015

The PRs you linked in your initial comment(#285 and #303). I'm not sure if those changes are in the 5.2.1 branch.

@mgartner

This comment has been minimized.

Copy link
Contributor Author

mgartner commented May 19, 2015

Ahh I see. Ya it looks like those changes made it into 5.1.2 (last two in the image):

screenshot 2015-05-19 13 44 45

@AdriVanHoudt

This comment has been minimized.

Copy link
Contributor

AdriVanHoudt commented May 19, 2015

@mgartner bump the issue by asking where he stands on the matter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.