Manager - Code Hygiene #312
Conversation
- Move ABI functions to common package - Validate Body and Query parameters with express-validator - Remove 'as any' from 'req.body' (There is a relative import of rpch/common because of file needed)
There was a problem hiding this comment.
@diegoalzate are there other areas where we can re-use these ABI functions?
i dont think so, the functions seen in the |
There was a problem hiding this comment.
LGTM
can @diegoalzate also check this to make sure moving the hardhat file will not cause any problems? E2E test worked so I think it's ok
There was a problem hiding this comment.
It looks like it will work. I would rename the abi file in commons to blockchain, seems like a lot is happening in that file or maybe even split it up into abi.ts and blockchain.ts. Also I saw that the function sendTransaction does not receive abi through params and receives it globally, i would update those type of functions to receive the abi through params and that way it can be easier to test.
|
@diegoalzate I agree with the name change |
|
@ItsLuized name change is pending here |
This pull request improves the code quality of manager.
To make the code hygiene of
manageras it was discussed in arch week n1, the following fixes were done:(There is a relative import of rpch/common because of file needed)
Closes #304