Upgrade challenge 0 to v3 #150
Conversation
danielcdz
force-pushed
the
upgrade-challenge-0-to-v3
branch
from
b4dc23f to
ed39327
Compare
| @@ -0,0 +1,221 @@ | |||
| use starknet::ContractAddress; | |||
There was a problem hiding this comment.
It should not look like this, I expect to see what section of the code has been changed/improved in order to match with latest dependencies
This file should not change from empty file to new implementation, we have already a working contract
There was a problem hiding this comment.
I believe it looks like this because I started the migration using the base-challenge branch and added the necessary updates from challenge-0. However, the implementation is the same as what we currently have in challenge-0
| @@ -0,0 +1,58 @@ | |||
| #[starknet::contract] | |||
There was a problem hiding this comment.
Do we really need mock_contracts folder? As It is mentioned in the issue description we expect to remove this folder if it is not needed
| @@ -1,5 +1,13 @@ | |||
| mod YourContract; | |||
There was a problem hiding this comment.
mod YourContract is not part of latest stable release
I dont expect to see this change
There was a problem hiding this comment.
Yes, I deleted this since the base-challenge branch has YourContract but as you mentioned for challenge-0 the mod is YourCollectible
There was a problem hiding this comment.
Hi! Dont need to delete a file thats has never been on this challenge-0. As you say I expected to see whats has been changed/improved on YourCollectible
| openzeppelin = { git = "https://github.com/OpenZeppelin/cairo-contracts.git", tag = "v0.14.0" } | ||
| snforge_std = { git = "https://github.com/foundry-rs/starknet-foundry", tag = "v0.27.0" } | ||
| starknet = "2.8.2" | ||
| openzeppelin = "0.17.0" |
There was a problem hiding this comment.
Don't need to use full OZ, lets import the specific modules we need for this challenge
| mod YourCollectible { | ||
| use starknet::storage::Map; | ||
| use contracts::components::Counter::CounterComponent; | ||
| use contracts::components::ERC721Enumerable::ERC721EnumerableComponent; |
There was a problem hiding this comment.
I expect to get rid of this custom ERC721EnumerableComponent and use the provided by OZ
|
@gianalarcon I'll be addresing your comments asap 🫡 ty for your feedback |
|
Hi @danielcdz Not sure if you can finish it on time, anyway the issue will remain open even after OD8 hacks end. Thanks! Do your best! |
|
closing this PR to change the approach, following advice from @gianalarcon , I'll be opening a new one asap |
Full update challenge-0 to v3
Types of change
Comments (optional)