search

zmb3 / spotify

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

Remove dependency on env #122

Closed pixelrazor closed 4 years ago

pixelrazor commented 4 years ago

NewAuthenticator shouldn't depend on the os environment; how those are supplied should be up to the user.

Removed the method to explicitly set those values. NewAuthenticator shouldn't return an invalid object that has to be configured with another function

zmb3 commented 4 years ago

I get your point, but this is largely subjective and breaks existing users. If we’re going to break things for people and cause them extra work to upgrade, there should be a really good reason. Is there something that’s not possible in the current implementation that this PR enables?

pixelrazor commented 4 years ago

I get your point, but this is largely subjective and breaks existing users. If we’re going to break things for people and cause them extra work to upgrade, there should be a really good reason. Is there something that’s not possible in the current implementation that this PR enables?

Nope, i wasn''t expecting a merge. I just wanted to put thoughts out for consideration if there's ever a v2