Skip to content
This repository has been archived by the owner on Jul 3, 2021. It is now read-only.

getPlaylists callback argument is not a RESTCallback #146

Closed
mortenn opened this issue Oct 17, 2017 · 6 comments
Closed

getPlaylists callback argument is not a RESTCallback #146

mortenn opened this issue Oct 17, 2017 · 6 comments

Comments

@mortenn
Copy link
Contributor

mortenn commented Oct 17, 2017

The callback argument for getPlaylists is of the form (data) => {}, not (err, data) => {} as is the norm for other methods in the API.

@thedark1337
Copy link
Member

thedark1337 commented Oct 17, 2017

yes, because its stored in a buffer object that doesn't result in an error. Its not the only function that isnt in the (err, data) => {} format.

getHistory
getActivePlaylist
getPlaylist
getPlaylists
getJar
(Maybe a few others)

These all don't use API methods to send a REST request and are documented to have the standard function callback style. All of them are cached in their respective locations in plugAPI itself.

@mortenn
Copy link
Contributor Author

mortenn commented Oct 17, 2017

I kind of like it when APIs are consistent, would you be opposed to change them?
As a better alternative, if they returned a Promise, I wouldn't need to wrap them at all :)

@thedark1337
Copy link
Member

Changing them would break any user's code that uses callbacks unfortunately. I can't just change it to return callback(null, data);

@mortenn
Copy link
Contributor Author

mortenn commented Oct 18, 2017

Returning a promise shouldn't break anyone, I think?

@samhmills
Copy link
Member

samhmills commented Oct 18, 2017

Changing to promises requires a large discussion on the direction of plugapi

@thedark1337
Copy link
Member

Regarding the discussion on promise usage, that will be moved to #144.
As for this issue, RESTCallback as its named is in the form of error, data for API REST calls.

For those that are just pure setters / getters with no interaction with the REST API, those only return callback(data) or data.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants