search

zmb3 / spotify

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

Accept collaborative parameter for CreatePlaylistForUser #158

Closed samuelrey closed 3 years ago

samuelrey commented 3 years ago

The collaborative parameter is optional in the Spotify Web API, and was not supported in this library. I add the parameter to directly to the CreatePlaylistForUser function, though a case could be made for writing a separate function in order to avoid breaking existing calls.

Note that I do not check for the relationship between public and collaborative in the body of the function, instead I let the Web API return an error.

I added two tests for the private/public-collaborative permutations in order to demonstrate the intended usage of the fields.

strideynet commented 3 years ago

Hi ! Thanks for this PR, I may have to move this to the V2 branch or similar since I'm trying not to introduce breaking changes in v1. I will think about this today and review it tonight.

samuelrey commented 3 years ago

Sounds good! Let me know if I need to update the changes or the PR.

strideynet commented 3 years ago

Hey sorry for the late response, I think we will need to introduce a separate function here for v1 until we can break this in v2.

samuelrey commented 3 years ago

No worries. Just updated the PR. I decided to copy-paste the body of CreatePlaylist because my understanding is that the plan is to have a single CreatePlaylist function instead of multiple. If the plan is to have multiple, then there's a case for deduping the core logic. Let me know what you think!

samuelrey commented 3 years ago

Also let me know if you want me to open a separate PR against V2 with the updated param list.

strideynet commented 3 years ago

Also let me know if you want me to open a separate PR against V2 with the updated param list.

@samuelrey Yeah, I think for v2 I want to move to an options struct or similar, rather than a parameter list. It'll make changes like this less breaking.

samuelrey commented 3 years ago

Sweet! Thanks for the approval. Please let me know when you merge the changes. I've got a project that depends on this behavior.

Yeah, I think for v2 I want to move to an options struct or similar, rather than a parameter list. It'll make changes like this less breaking.

Nice! Feel free to reach out if you could use some help on those efforts.