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
Exclude nil Response from response body #41
Conversation
Message: msg, | ||
Errors: errors, | ||
})) | ||
} | ||
|
||
// A struct with the fields that we need from github.ErrorResponse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @stoneman , I'm sorry it has taken quite a while for me to get around reviewing this.
I will have a look at this later today. Thanks for contributing!
Hi @stoneman , I think I've caught up with this issue. It took me a while to understand it once again. I think you've nailed the problem and I also checked that creating this simplified error response does mitigate the current problem of using the errors. Have said that, now that we know where the problem comes from, I've found suuuuper weird that the Is that possible that this is just an issue with the SDK itself? With that I'm happy to mitigate the problem with your fix but I reckon we should take this to the folks in the golang github sdk repo. Any thoughts? |
There is no Presumably, other types of errors (none of which ever have Maybe I should change the name of the struct from |
That's exactly the point. Since the
Exactly. Github itself never returns the attribute
I still think the problem is in the SDK and |
I've added a small PR to I reckon if that gets merged to master, it's a win and we should fix this issue. If, for some reason we are unable to merge it to master, we can then mitigate the issue with this new |
Ah, I see - thanks for getting this fixed 🙂 |
Fixes: #6