Skip to content

Clashing items on the reviewer and crafter checklists #37

@oddaf

Description

@oddaf

The mainnet Spell reviewer and crafter checklists do not agree regarding the use of dss-interfaces.

On the reviewer checklist the use of dss-interfaces is mandatory, the CL states it SHOULD be used unless the function call is not present there:

  • Static Interfaces
    • No unused static interfaces
    • Declared static interface not present in the dss-interfaces, OTHERWISE should be imported from there

On the crafter checklist, however, there are other situations in which the use of a static interface is acceptable:

  • IF some actions require using interfaces
    • Prefer using DssExecLib actions where possible (to avoid adding interfaces where not required)
    • Avoid multi-import layout / importing from Interfaces.sol (see issue #69)
    • Prefer single import layout (e.g. import { VatAbstract } from "dss-interfaces/dss/VatAbstract.sol";)
    • Use static interfaces IF not present in dss-interfaces OR present in dss-interfaces but outdated OR only a few function interfaces are needed

Since we rarely call more than a few functions in a contract, the crafter CL pushes the crafter to go against the review item, creating unnecessary friction during the review process. Both CLs should be aligned and push for a single pattern.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions