Skip to content

Conversation

@Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Oct 12, 2022

The term "collectible" has been replaced by "NFT" everywhere. The term "collectible" was originally used because it was thought NFT would be more difficult for users to understand, but today in practice we all use the term NFT. Efforts to use the term "collectible" have long been abandoned.

  • BREAKING:

    • Rename CollectiblesController to NftController, and rename all methods, controller properties, types, constants, and state properties to use the term "NFT" instead of "collectible".
    • Rename CollectibleDetectionController to NftDetectionController, and rename all methods, controller properties, types, constants, and state properties to use the term "NFT" instead of "collectible".
    • Rename AssetsContractController method getERC721CollectibleTokenId to getERC721NftTokenId.
    • Rename assetsUtil method compareCollectiblesMetadata to compareNftMetadata.
    • Rename constant ASSET_TYPES.COLLECTIBLE to ASSET_TYPES.NFT
    • Rename PreferencesController state property useCollectibleDetection to useNftDetection
    • Rename PreferenceController method setUseCollectibleDetection to setUseNftDetection

Checklist

  • Tests are included if applicable
  • Any added code is fully documented

@Gudahtt Gudahtt force-pushed the rename-collectible-to-nft branch 3 times, most recently from 9127a01 to 93828a4 Compare October 12, 2022 18:10
@Gudahtt Gudahtt marked this pull request as ready for review October 12, 2022 18:21
@Gudahtt Gudahtt requested a review from a team as a code owner October 12, 2022 18:21
adonesky1
adonesky1 previously approved these changes Oct 12, 2022
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

Didn't review every single line but LGTM!

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Noticed one typo but otherwise this looks good to me.

The term "collectible" has been replaced by "NFT" everywhere. The term
"collectible" was originally used because it was thought NFT would be
more difficult for users to understand, but today in practice we all
use the term NFT. Efforts to use the term "collectible" have long been
abandoned.
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit bdf76a6 into main Oct 13, 2022
@Gudahtt Gudahtt deleted the rename-collectible-to-nft branch October 13, 2022 18:09
@mcmire mcmire mentioned this pull request Oct 13, 2022
37 tasks
gantunesr pushed a commit that referenced this pull request Dec 8, 2022
The term "collectible" has been replaced by "NFT" everywhere. The term
"collectible" was originally used because it was thought NFT would be
more difficult for users to understand, but today in practice we all
use the term NFT. Efforts to use the term "collectible" have long been
abandoned.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The term "collectible" has been replaced by "NFT" everywhere. The term
"collectible" was originally used because it was thought NFT would be
more difficult for users to understand, but today in practice we all
use the term NFT. Efforts to use the term "collectible" have long been
abandoned.
MajorLift pushed a commit that referenced this pull request Oct 11, 2023
The term "collectible" has been replaced by "NFT" everywhere. The term
"collectible" was originally used because it was thought NFT would be
more difficult for users to understand, but today in practice we all
use the term NFT. Efforts to use the term "collectible" have long been
abandoned.
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.

4 participants