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

License View with medias #173

Merged
merged 5 commits into from
Aug 28, 2023
Merged

License View with medias #173

merged 5 commits into from
Aug 28, 2023

Conversation

joshuahannan
Copy link
Member

@joshuahannan joshuahannan commented Jun 26, 2023

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
Copy link
Member Author

I feel good about this view for now. We just need a couple more things before we can merge this and announce the deployment.
@albeethekid is going to:

  • Add a page in the docs site about the license project and how to use the metadata view
  • Update the README here

Then we need to get a few approvals on this PR

Copy link
Collaborator

@sisyphusSmiling sisyphusSmiling left a comment

Choose a reason for hiding this comment

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

Really exciting to see this coming along! Also really dig the creational pattern.

@@ -159,7 +160,8 @@ pub contract ExampleNFT: NonFungibleToken, ViewResolver {
traitsView.addTrait(fooTrait)

return traitsView

// case Type<MetadataViews.NFTLicense>():
// return nil //MetadataViews.NFTLicense()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we know the license we'll use for this example contract? As long as things are commented, it would be nice to demo the intended use of the helper methods.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated it to include the nlp util view. I had it commented out because I was seeing a bug with the cadence testing framework

contracts/MetadataViews.cdc Show resolved Hide resolved
@albeethekid albeethekid merged commit 557af45 into master Aug 28, 2023
2 checks passed
@joshuahannan
Copy link
Member Author

This was mistakenly merged. I reverted this in a different PR and I'm going to open a new PR with these changes so we can continue to get reviews and add docs

@joshuahannan joshuahannan deleted the license-view branch August 28, 2023 13:36
@joshuahannan
Copy link
Member Author

I opened a new PR with the same changes. Everyone who has approved already please add their approval to that PR and we'll work on getting the docs updated and getting more eyes on the PR.

#186

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

Successfully merging this pull request may close these issues.

5 participants