Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add Lidarr and MusicBrainz support #782

Closed
wants to merge 1 commit into from

Conversation

Svagtlys
Copy link

This PR aims to add Lidarr and MusicBrainz support to Jellyseerr to address feature request #96

I'm pretty new to this kind of coding, so I'm mostly just making changes, running a build, and fixing bugs as they come up. I'm using sct/overseerr#3800 by @anon0002 as a guide/code source for the heftier bits like the Lidarr API classes.

In feature request #96 @zachbakerdev mentioned using Deezer instead of MusicBrainz as it has more info on covers, but I'm not sure that Lidarr supports that and I've not interacted with Deezer before, so I'm currently sticking with MusicBrainz. I'm happy to also add Deezer support at some point in the future.

…class

Edited the Jellyfin API hooks to allow accessing music library items, and track musicbrainz ids.
Also added the Lidarr API related classes from Overseerr PR #3800 by @ano0002

re Fallenbagel#96
@zachbakerdev
Copy link

zachbakerdev commented May 31, 2024

One of the other considerations I had was instead of using the cover art archive that Musicbrainz uses was searching on Musicbrainz for the albums, then mapping those albums to Deezer instead of the other way around. Just a consideration.

Guarantees the album is available in Lidarr, plus high quality cover arts.

@ano0002
Copy link

ano0002 commented Jun 3, 2024

It seems like you forgot to add the type definitions I had to add so it will not work I think.
I'm talking about this
But since I don't know how jellyseer is setup you might not need it

@Svagtlys
Copy link
Author

Svagtlys commented Jun 4, 2024

@ano0002 I actually had a few questions about your implementation of the nodebrainz API, for example why didn't you copy the interfaces of TMDB with all of the Result interfaces?

I haven't added all of your work into this PR (yet) as I'm trying to learn a bit about Typescript/Javascript and how different people code as I'm doing this :D I'll admit I also don't have a deep understanding of how Jellyseerr functions so I'm trying to learn that as well to determine which nodebrainz features are needed.

@ano0002
Copy link

ano0002 commented Jun 4, 2024

@ano0002 I actually had a few questions about your implementation of the nodebrainz API, for example why didn't you copy the interfaces of TMDB with all of the Result interfaces?

I haven't added all of your work into this PR (yet) as I'm trying to learn a bit about Typescript/Javascript and how different people code as I'm doing this :D I'll admit I also don't have a deep understanding of how Jellyseerr functions so I'm trying to learn that as well to determine which nodebrainz features are needed.

I'll try to answer all those question when I get some time as right now I have all my final exams.

Copy link

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@github-actions github-actions bot added the merge conflict Cannot merge due to merge conflicts label Jun 13, 2024
@ano0002
Copy link

ano0002 commented Aug 31, 2024

@ano0002 I actually had a few questions about your implementation of the nodebrainz API, for example why didn't you copy the interfaces of TMDB with all of the Result interfaces?

I haven't added all of your work into this PR (yet) as I'm trying to learn a bit about Typescript/Javascript and how different people code as I'm doing this :D I'll admit I also don't have a deep understanding of how Jellyseerr functions so I'm trying to learn that as well to determine which nodebrainz features are needed.

I didn't copy them because I didn't want to send the same kind of info. I could have reused it and modified it to take more parameters but that would have meant I'd have to rewrite every other time when the interface was already used so I chose to make a separate one.

@Svagtlys Svagtlys closed this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge conflict Cannot merge due to merge conflicts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants