search

RustyNova016 / musicbrainz_rs_nova

A wrapper around the musicbrainz API
MIT License
5 stars 3 forks source link

feature: add possibility to look up disc with discid #39

Closed eknoes closed 2 months ago

eknoes commented 2 months ago

I missed to possibility to look up a disc in my drive through the DiscId. With that patch it is possible. It seemed to me the disc entity was not used before.

Discid::fetch().id(&disc_id.id()).execute()
eknoes commented 2 months ago

Cool, I'll do the changes in the next days :) Yes, its pretty basic, I just changed what I needed for my usecase and never worked with the library before. I'll do the changes and write a test or two!

RustyNova016 commented 2 months ago

Same as me. I started with only for my use case and it grew so much now I'm maintaining the crate -_-'

For tests there's the blocking and async ones. Be careful as async is on by default, and you need to specify to use blocking. There's a line in the cargo.toml to easily switch defaults for development

eknoes commented 2 months ago

I tested some discids and wrote a test! :)

RustyNova016 commented 2 months ago

There's only the three fields left to make non optional (see review above). Also don't forget to do cargo fmt. There's a CI check for that

eknoes commented 2 months ago

Sorry, I don't see what three fields you mean but I guess the ones I changed now? Also did the formatting.

RustyNova016 commented 2 months ago

One last thing I didn't caught earlier is that the base branch was main instead of develop... And sadly there's a merge conflict with the serialization. I'll merge it into a new branch and resolve it myself.

eknoes commented 2 months ago

Thanks a lot! Sorry, did not know of the develop branch, next time I know :)

RustyNova016 commented 2 months ago

I should just do better documentation too... Apparently gitflow is not universally know