onchainification / robosaver

🤖 RoboSaver turns your Gnosis Pay card into an automated savings account!
https://robo-frontend-blue.vercel.app/
GNU General Public License v3.0
8 stars 2 forks source link

feat: reorg constants and errors into its own `.sol` file #70

Closed petrovska-petro closed 3 months ago

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.

Project coverage is 97.19%. Comparing base (c2eb4ff) to head (ba5f024). Report is 1 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #70 +/- ## ========================================== - Coverage 97.74% 97.19% -0.55% ========================================== Files 2 2 Lines 177 178 +1 Branches 30 30 ========================================== Hits 173 173 - Misses 1 2 +1 Partials 3 3 ``` | [Files](https://app.codecov.io/gh/onchainification/robosaver/pull/70?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onchainification) | Coverage Δ | | |---|---|---| | [src/RoboSaverVirtualModuleFactory.sol](https://app.codecov.io/gh/onchainification/robosaver/pull/70?src=pr&el=tree&filepath=src%2FRoboSaverVirtualModuleFactory.sol&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onchainification#diff-c3JjL1JvYm9TYXZlclZpcnR1YWxNb2R1bGVGYWN0b3J5LnNvbA==) | `91.66% <66.66%> (ø)` | | | [src/RoboSaverVirtualModule.sol](https://app.codecov.io/gh/onchainification/robosaver/pull/70?src=pr&el=tree&filepath=src%2FRoboSaverVirtualModule.sol&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=onchainification#diff-c3JjL1JvYm9TYXZlclZpcnR1YWxNb2R1bGUuc29s) | `98.05% <80.00%> (-0.65%)` | :arrow_down: |
petrovska-petro commented 3 months ago

im guessing you are not combining both constants into one file because it will increase the bytesize of both contracts unnecessarily?

feel that it's cleaner and better org for the constants to have their own file for each its own scope

gosuto-inzasheru commented 3 months ago

so i think it would be nice to also import all constants into base fixtures, and thus prevent having to redefine constants that are already defined.

this also makes sure that if we change something in the module that it immediately is reflected in the tests as well

gosuto-inzasheru commented 3 months ago

i remember us talking about then also not having to make the constants public.

rn they are only public so that they can be retrieved easily in the tests i think

petrovska-petro commented 3 months ago

so i think it would be nice to also import all constants into base fixtures, and thus prevent having to redefine constants that are already defined.

see 30f3ffdfd2eeae8afca13c080d46877511073e34