Removed unused imports and return statement, added a test

[RyanPaulGannon]

Removed a duplicate in the config solidity-coverage, removed the return s_tokenCounter; in the mintNft(). Added the constructor tests.

[PatrickAlphaC]

Can we just remove the duplicate and keep the tests as-is without removing the return s_tokenCounter?

[RyanPaulGannon]

I'll close this because I've revisited this and realised I've rushed over this lesson. Running hh coverage with the tests as they are shows 100% so I just got caught up in using the tests from the ERC20. Also, after playing around with the BasicNFT.sol contract again, I realised that the following mintNFT() function uses more gas than removing the return statement.

function mintNFT() public returns (uint256) {
        // Every time a new NFT is minted, we increase the token counter
        s_tokenCounter = s_tokenCounter + 1;
        _safeMint(msg.sender, s_tokenCounter);
        return s_tokenCounter;
    }

So, if I submit a PR with an update to the mintNFT() function,

function mintNFT() public {
        // Every time a new NFT is minted, we increase the token counter
        s_tokenCounter = s_tokenCounter + 1;
        _safeMint(msg.sender, s_tokenCounter);
    }

this, would be better as its more gas efficient?

[PatrickAlphaC]

Yes, you're right, it would be more gas efficient. But since I left it in the video, it's a little easier to keep the code the same as what's in the video. Since the function currently isn't wrong.

But yes I agree, in production, we'd remove that return most likely.

[RyanPaulGannon]

Ah that's cool, just glad I understand it a little bit more! Thanks for clarifying, I shall close this now.