search

public-awesome / cw-nfts

Examples and helpers to build NFT contracts on CosmWasm
Apache License 2.0
188 stars 180 forks source link

Is `ownership(deps: Deps)` really needed? #133

Closed taitruong closed 1 month ago

taitruong commented 1 year ago

From what I see, ownership(deps: Deps) is only used in one place (query): https://github.com/CosmWasm/cw-nfts/blob/177a993dfb5a1a3164be1baf274f43b1ca53da53/contracts/cw721-base/src/query.rs#L321-L336

So do we need to make this public? If so, maybe it is better re-exporting from ownable, like this:

// re-export to make it easier for anyone using this, without using ownable directly
pub use cw_ownable::get_ownership;
taitruong commented 1 year ago

After checking it again, this is a bit confusing: https://github.com/CosmWasm/cw-nfts/blob/177a993dfb5a1a3164be1baf274f43b1ca53da53/contracts/cw721-base/src/msg.rs#L155

Since v0.17.0 cw-ownable replaces minter. So QueryMsg for Minter and Ownership is the same thing. So either minter should be noted as deprecated or one of these 2 should be removed.

taitruong commented 1 month ago

solved in v19