Clashing items on the reviewer and crafter checklists
Opened this issue · 0 comments
oddaf commented
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 indss-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.