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

CIP-0060 | Update to v2 #502

Merged
merged 1 commit into from
Jun 6, 2023

Conversation

AndrewWestberg
Copy link
Contributor

No description provided.

@rphair rphair changed the title Update cip-0060 to v2 CIP-0060 | Update to v2 Apr 10, 2023
@rphair rphair added the Update Adds content or significantly reworks an existing proposal label Apr 10, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sound to me (and anticipated in the CIP and last year's discussion of it) but I'm not qualified to review the metadata particulars. @AndrewWestberg your authority would be good enough for me though also tagging @sean118 from the original CIP review for a third party perspective & would also welcome anyone working in this field to sign off on this. 🤓

@AndrewWestberg
Copy link
Contributor Author

@rphair I'm having those in the original discussions from last year look over this as well so there's no hurry to merge this. The suggested changes came from Jimmy Londo at SickCity. I want him to look over it as well to make sure I've captured exactly what he wanted to do with it.

@rphair rphair added the Category: Metadata Proposals belonging to the 'Metadata' category. label Apr 11, 2023
@AndrewWestberg
Copy link
Contributor Author

I meet with Jimmy on Thursday to discuss the changes. So... no merging for now.

@AndrewWestberg
Copy link
Contributor Author

@rphair Changes have been discussed and finalized with stakeholders. This can be merged whenever.

Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the status @AndrewWestberg - marking Last Check since I believe with your endorsement this is would be a quick review item at our next CIP meeting in 3 days' time before merging... have asked the convenor to add it to the agenda.

@rphair rphair added the State: Last Check Review favourable with disputes resolved; staged for merging. label Apr 15, 2023
@sean118
Copy link

sean118 commented Apr 15, 2023

Hi, thanks for tagging and sorry for the late response.

Would like to drop a few thoughts on all of this.

Firstly, the document is quite hard to read, because you're nesting many of the fields into various objects without revealing it in the overview table.

It might make sense to label a field "release.release_type" if it's nested in that structure. Then it also shows that it might make sense to just call it "release.type" and "release.title" instead, saving some space.

On contributors / people metadata:

For "Artists" there is an object, which is great, because it can easily be extended with socials / DIDs etc. to make the artist uniquely identifiable. But for other contributors involved (mix / mastering engineers, producers, visual artists etc.) there's only a string - why not use something like a contributor-object for all so they can also add own social handles, links etc. if they want? For players / recommendation algos etc. there's no way to uniquely identify them otherwise once there's two contributors with the same name.

Secondly, some of them appear in the new "release" object, while others, or only the artists (?), appear in the song objects. Along with "copyright", "genres" and other song-related fields, the artists should also be mentionable in the "release" object and vice versa. Some songs of an album may have been produced by different producers. And this would also remove loads of duplication seen in the "Multiple" example.

On release types:

I think it'd make a lot of sense to stick with the industry standards: Single / EP, Album / LP, Compilation, Mixtape etc. and not reinvent the wheel with a "Multiple" release type. The release types make it possible to easily distinguish between "appearances" of artists on compilations / collections and an artist's own albums and other release types. I think it's a basic feature we're used to in modern music streaming services and players should be able to categorize releases accordingly.

On the "song" object nested inside the file object:

This structure seems a bit bloated to me. I would suggest adding a "files[*].type = song" (consistent with the release.type) and then just adding song-related attributes directly to the file object.

Also, the "files[].song.song_title" is identical with the "file[].name" in all examples, so I think "song_title" can simply be removed.

On genres:

I think the limit is too narrow for many use cases. While you may just want to display the basic top-level genres (rock, pop, hiphop etc.) in your initial product's UI, under the hood for recommendation systems additional genres can be very useful. Also the examples imo demonstrate that there's a need for more granular genres (you may not have a "Guitar Live Looping" section in your app, but if an algo comes across this again it may find a great recommendation). Why limit this at all, when you can decide to just ignore them for your particular product?

Hope there's some value in this; maybe also just for the next v3 iteration.

Best regards,
Sean

@rphair
Copy link
Collaborator

rphair commented Apr 18, 2023

@AndrewWestberg looks like this is ready for last check, but also our feeling at the meeting is that there could be documentation improvements that would make it more readable as suggested like @sean118 ... so we wanted to give you more time to respond to those suggestions & otherwise this seems ready to merge.

@AndrewWestberg
Copy link
Contributor Author

Let's go ahead and merge for now. We can address the @sean118 comments in v3.

@rphair
Copy link
Collaborator

rphair commented May 17, 2023

@KtorZ @SebastienGllmt @Ryun1 somehow the Last Check tag didn't get this on the agenda for yesterday's meeting. Can we review & merge it here, at least before the next sort-of-biweekly update with the CIP number assignments & table updates?

@rphair
Copy link
Collaborator

rphair commented Jun 6, 2023

@AndrewWestberg confirmed for Last Check at meeting today: https://hackmd.io/@cip-editors/67

@rphair rphair merged commit 5405c9f into cardano-foundation:master Jun 6, 2023
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Jul 28, 2023
Ryun1 pushed a commit to Ryun1/CIPs that referenced this pull request Nov 17, 2023
@rphair rphair removed the State: Last Check Review favourable with disputes resolved; staged for merging. label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Metadata Proposals belonging to the 'Metadata' category. Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants