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

fix(nft): switch nft traits to not require moving underlying struct #475

Merged
merged 6 commits into from
Jul 22, 2021

Conversation

austinabell
Copy link
Contributor

@austinabell austinabell commented Jul 15, 2021

  • Pointed out by @mattlockyer as an API with friction when customizing, there was no need to move the struct in the first place
  • Also switched enumeration standard to not require moving self either

I also bumped the version so that I can make the minor release after it comes in can't actually release this change unless I cherry-pick on top of the previous version, this contains other changes not released

@austinabell austinabell changed the title fix(nft): nft_token method switched to only take reference to token struct fix(nft): switch nft traits to not require moving underlying struct Jul 15, 2021
@ChaoticTempest
Copy link
Member

We'll likely need to update the near-examples/NFT example after this, since users will probably be using the example as a baseline. So better to update it sooner than later I believe

@austinabell
Copy link
Contributor Author

We'll likely need to update the near-examples/NFT example after this, since users will probably be using the example as a baseline. So better to update it sooner than later I believe

I'm not certain, but I think all of these functions are generated with the impl macro so won't need to change anything unless you override methods. I could be wrong though

@ChaoticTempest
Copy link
Member

I'm not certain, but I think all of these functions are generated with the impl macro so won't need to change anything unless you override methods. I could be wrong though

Oh if its in the macro, then it should be fine

Copy link
Contributor

@mikedotexe mikedotexe left a comment

Choose a reason for hiding this comment

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

I wasn't quite sure of the motivation to not use & and prefer this approach. I think perhaps there was an idea of having self be captured, but think this is in line with what we have elsewhere. Thanks

@mikedotexe mikedotexe merged commit e379227 into master Jul 22, 2021
@mikedotexe mikedotexe deleted the austin/nft_token_int branch July 22, 2021 15:16
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.

3 participants