search

oknozor / musicbrainz_rs

A wrapper around the musicbrainz API
MIT License
38 stars 18 forks source link

docs: cover more attributes #68

Closed ritiek closed 3 years ago

ritiek commented 3 years ago

I've also been introducing new enums for the fields which can have only have one specific value assigned to them from the multiple values known beforehand.

ritiek commented 3 years ago

Alrighty, this is ready for review.

There are still many remaining entity fields which I haven't covered in this PR mainly because they aren't properly mentioned in the MusicBrainz docs either. For example, in the Artist entity, we have fields named rating and country, but they aren't mentioned anywhere in the Artist doc page and it also seems like doc pages for Rating and Country docs do not exist (unlike the Artist page to which I linked, exists). I think we'll probably have to think and write a docstring for such fields ourselves. There are a lot of such undocumented fields when considering all other entities (not just in the Artist entity) and this will probably take a lot of work which I think is out of scope for this PR.

So, for this PR; I've mainly copy-pasted doc strings for the fields for which MusicBrainz already have one. I've also introduced a few type enums and added FIXMEs for the ones that could be considered too.

oknozor commented 3 years ago

This is already a great improvement! We should probably ask the musicbrainz community about the undocumented fields.

oknozor commented 3 years ago

There is a conflict and I am merging this from my phone,let me know when it's fixed :)

ritiek commented 3 years ago

@oknozor Yup. I just resolved it so should be good now.