zmb3 / spotify

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

Fixing CurrentUsersTracksOpt market query parameter + proper handling of results #138

Closed fclairamb closed 3 years ago

fclairamb commented 3 years ago

Thank you for this great lib.

Updating all the API to support track relinking:

fclairamb commented 3 years ago

Hi @zmb3, so what is the next step? Do you want me to change something or can we merge it as-is?

zmb3 commented 3 years ago

Sorry @fclairamb I didn't see your reply 😬

The web API docs show that the following calls support track relinking:

It looks like "country" is not valid for any of those, so I am fine overriding "country" in these cases to be used for track relinking, so long as the godoc makes that clear.

I do think it makes sense to get all of these calls supporting track relinking together though, rather than leave things in a state where relinking works for some things but not others. Are you up for making the same changes in all of these places? I don't mind if it's part of the same PR or separate ones, so long as they all get updated in the same 1-2 days.

fclairamb commented 3 years ago

Wowkay, I guess I can do that. I indeed only focused on fixing the parts where I needed this feature.

fclairamb commented 3 years ago

Hi @zmb3, so I finally took the time to update the other parts of the library where the market parameter can be used.

Please note I broke the GetAlbumTracksOpt API because it's the only one of the *Opt function that didn't receive an *Option parameter. I think it's worth it here.

zmb3 commented 3 years ago

Thanks @fclairamb for taking the time, and happy new year!

fclairamb commented 3 years ago

Great! Thank you and happy new year! 🎉