search

zmb3 / spotify

A Go wrapper for the Spotify Web API
Apache License 2.0
1.37k stars 287 forks source link

extend expected http.StatusAccepted with http.StatusNoContent (closes #99) #100

Closed Eun closed 4 years ago

Eun commented 5 years ago

closes #99

Eun commented 5 years ago

Will fix these tests asap.

Eun commented 5 years ago

Ready to merge

zmb3 commented 5 years ago

Can we just make execute smart enough to treat all 2XX codes as successful so we don't have to pass http.StatusAccepted to every call?

Eun commented 5 years ago

Accepting all 200 codes feels a bit risky, instead I would rather just add the specified 200 codes from spotify:

https://developer.spotify.com/documentation/web-api/#response-status-codes

zmb3 commented 5 years ago

Sure, but the reason we're in this situation is that Spotify sometimes sends 200 codes that it doesn't document, and thus this library issues an error when the operation succeeded..

Eun commented 5 years ago

I haven't seen 200 codes that are undocumented. I can extend execute to treat 200, 201, 202 and 204 as success like they say here.

zmb3 commented 4 years ago

@Eun any follow up on extending execute as mentioned above?