search

jzheng2017 / spotify-web-api-wrapper

Spotify API wrapper for Java
MIT License
50 stars 32 forks source link

Make Spotify interfaces return actual paging objects #9

Open jzheng2017 opened 3 years ago

jzheng2017 commented 3 years ago

Is your feature request related to a problem? Please describe. Currently some api calls of the SpotifyApi interface return an object wrapping a Paging<T> object. This is done because of how the Spotify Web API return their json.

For instance PlaylistSimplifiedPaging is just wrapping around Paging<PlaylistSimplified>. The end user would have to make an additional getter call to get the actual contents of the paging object. This is unnecessary because it is an internal detail of how the json is mapped.

The following functions need to be changed:

If there are anymore I've missed, please comment it on this issue.

Describe the solution you'd like It would be better if the wrapped object contents got returned instead of the wrapper object itself.

So instead of returning the wrapped object, for instance PlaylistSimplifiedPaging, it would be better to returned the actual Paging object.

Describe alternatives you've considered None

Additional context The feature is actually already done once in FollowApiRetrofit class for the function getFollowedArtists. Take this function as reference when implementing the feature.

Pratik-Nataraj516 commented 3 years ago

Hi, is this something I can work on?

jzheng2017 commented 3 years ago

Hi, is this something I can work on?

@Pratik-Nataraj516 Yes, of course. I'll assign this issue to you. Implement the feature in the feature/unwrap-paging-objects branch. When creating the pull request to the upstream repository, make sure to point it to the same branch.

Also do not forget to read the contribution guidelines before starting.

Good luck!

EDIT: also do not change the function directly, but make new function and deprecate the old function with @Deprecated annotation. The annotation must have the following information: deprecated since <version>, an explanation why it is deprecated and a link to the new function that has to be used. This is done so the change is backwards compatible.

jzheng2017 commented 3 years ago

@Pratik-Nataraj516 please confirm by 27-12-2020 that you're going to implement this feature. If there is no response by then, I'll have to remove you as assignee.

SumanohariR commented 11 months ago

is this issue Still open? if yes, can I work on it? I work as a backend dev and I think I can work on this.

jzheng2017 commented 11 months ago

is this issue Still open? if yes, can I work on it? I work as a backend dev and I think I can work on this.

Yeah, it's still open and you can work on this.

SumanohariR commented 11 months ago

Okay, Thank you for the confirmation. I will start working on this

jzheng2017 commented 10 months ago

Okay, Thank you for the confirmation. I will start working on this

Any update on the progress of this issue?

DanielVargaPublic commented 8 months ago

Could you please take a look at the pull request resolving this issue? I've updated it as requested and I have a question about what version number to use.

DanielVargaPublic commented 8 months ago

Thank you, I updated the version numbers. Could you please create a release branch into which I could open a new pull request?

jzheng2017 commented 8 months ago

Thank you, I updated the version numbers. Could you please create a release branch into which I could open a new pull request?

I have created a 2.0.0 branch. Please use the old PR and target that branch.

DanielVargaPublic commented 8 months ago

Thank you, I updated the version numbers. Could you please create a release branch into which I could open a new pull request?

I have created a 2.0.0 branch. Please use the old PR and target that branch.

Thank you. I updated the base branch of the PR and re-requested the review.

jzheng2017 commented 7 months ago

Thank you, I updated the version numbers. Could you please create a release branch into which I could open a new pull request?

I have created a 2.0.0 branch. Please use the old PR and target that branch.

Thank you. I updated the base branch of the PR and re-requested the review.

Cool. Will take a look this week.

DanielVargaPublic commented 7 months ago

Hi! Could you please check the PR?

jzheng2017 commented 7 months ago

https://github.com/jzheng2017/spotify-web-api-wrapper/pull/91 has been merged to /2.0.0 branch. This issue will be closed once the branch makes it to main.