zmb3 / spotify

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

feat: Enable option to not sent market using GetArtistAlbumsOpt #115

Closed brodin closed 4 years ago

brodin commented 4 years ago

https://github.com/zmb3/spotify/blob/master/artist.go#L127-L131

I have a use case where I want ALL albums for an artist and then I need to be able to not send the markets url-value. Edit: ~This is my suggestion on how to do it while keeping the current behaviour where an unspecified market will give you US as default.~

Suggestions for other implementations are welcome 🙏

zmb3 commented 4 years ago

Seems reasonable to me. Can we just document this behavior on the method docs?

brodin commented 4 years ago

Seems reasonable to me. Can we just document this behavior on the method docs?

Of course, the is fallback is however not documented. The more I read the less confused I get. If you pass in nil as options you don't get the options.Country = US default. I am thinking that we should simplify remove it and add the disclaimer(below) in the method documentation instead?

// if the market is not specified, Spotify will likely return a lot
// of duplicates (one for each market in which the album is available)
brodin commented 4 years ago

@zmb3 I force-pushed a new approach – what do you think?