-
Notifications
You must be signed in to change notification settings - Fork 363
Conversation
CLA Assistant Lite All Contributors have signed the CLA. |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
Pull Request Test Coverage Report for Build 1719914404
💛 - Coveralls |
Deployment links
|
E2E Tests Failed Failed tests:
|
It looks like dev departed from main a bit. You shouldn’t merge main into this branch. Dev is the base. |
4a7b07d
to
cfaf2ca
Compare
@@ -130,16 +148,16 @@ export const RemoveModuleModal = ({ onClose, selectedModulePair }: RemoveModuleM | |||
{(txParameters, toggleEditMode) => { | |||
return ( | |||
<> | |||
<ModalHeader onClose={onClose} title="Remove Guard" /> | |||
<ModalHeader onClose={onClose} title="Remove Module" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks great!
I'm a little worried about the added async-await. Are those calls all try-catch'ed somewhere?
Please test with failing transactions (e.g. super low gas).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
LGTM Enabled the module |
@germartinez could you solve the git conflict and merge this? Thanks! |
ESLint Summary View Full Report
Report generated by eslint-plus-action |
What it solves
Resolves #2797
How this PR fixes it
Uses the safe-core-sdk to call the methods
getEnableModuleTx
andgetDisableModuleTx
How to test it