Skip to content

Conversation

@drinkcoffee
Copy link
Contributor

** This is a draft at the moment - Solhint needs to be removed from the CI and all solhint comments need to be removed.

This PR resolves linter warnings. The warnings fall into the following categories:

Important changes - please review carefully:

  • Operator allowlist code had complex code in modifiers. The complex code would have been replicated for each function using the modifier. The updated modifiers call functions that have the same effect. This will decrease code size for situations when the the modifier is used multiple times in the one file.

Minor changes:

  • Use SCREAMING_SNAKE_CASE for constants.
  • Remove unused imports.
  • Stopped using global imports.

Globally disabled warnings:

  • mixed-case-function: Disable this so function names with ERC and URI are OK.
  • mixed-case-variable: Disable this so variable names with ERC and IRU are OK.
  • asm-keccak256: Disable this because existing usages of keccak256 will not be updated to use assembly. Additionally, it seems likely that the Solidity compiler will be updated at some point to take advantage of optimisations that could be done using in-line assembly.

Case-by-case disabled warnings:

  • unsafe-typecast: When a text constant was cast to bytes32 and used for a Access Control role.
  • pascal-case-struct: For structs in the ERC721 code.
  • incorrect-shift: For when in ERC 721 PSI2 1 is being shifted a variable number of times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants