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

NFT License View #186

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

NFT License View #186

wants to merge 4 commits into from

Conversation

joshuahannan
Copy link
Member

Description

Builds on the view proposed in #172 that is based on the Flow NFT License Project.

Includes Media links for a badge and a description.

The specific IPFS format, link type, and locations for the links are going to be delayed for now so we can get the basic licenses and their descriptions on chain.

I also removed the JS tests because they were causing issues. We're gonna try to use the Cadence testing framework here from now on.


For contributor use:

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the standards mentioned here.
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@joshuahannan joshuahannan changed the title add license changes to a new branch NFT License View Aug 28, 2023
@joshuahannan joshuahannan mentioned this pull request Aug 28, 2023
6 tasks
///
pub struct NFTLicense {
/// Array of the specific license identifiers
pub let licenses: [String]

Choose a reason for hiding this comment

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

maybe this can be renamed to 'rights' , we use rights terminology when adding into this.

Copy link
Member Author

Choose a reason for hiding this comment

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

great callout! I'll make that change

@bluesign
Copy link

nit: do we have plans to register licenses with spdx ? if so it would be nice to use existing License struct, instead of strings in the array.

@joshuahannan
Copy link
Member Author

@albeethekid Do you know the answer to the question about spdx?

@bluesign
Copy link

looks pretty good to me; but I have an idea; curious about what you think @joshuahannan

Now we have NFTLicense ( seems like this will be only used for NLP )

It has 5 building blocks:

access(contract) fun personalUse(): NFTLicense {
access(contract) fun votingRights(): NFTLicense {
access(contract) fun commercialRights(): NFTLicense {
access(contract) fun additionalContentExperienceRights(): NFTLicense {
access(contract) fun merchandisingRights(): NFTLicense {

We have 12 licenses. ( combination of these )

What if we take 5 blocks and making them public function (outside struct) , something like:

access(all) fun nlpVotingRights(): String{
    return "NLP-VOTE"  
}

and add a access(contract) setRights([String]) method or just take a string constructor to NFTLicense. (e.g. NLP-VOTE-COM ) depending on the license string we can set description, link, rights in the constructor directly. (with a switch maybe)

@joshuahannan
Copy link
Member Author

@bluesign What is the benefit of the solution you've proposed here over what we already have?

@bluesign
Copy link

actually not much benefit just making API a bit smaller. if we make spdx compatible later, we can just put spdx identifier, these views can be generated from that automatically etc. But for now not much difference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants