Closed karelianpie closed 2 years ago
Love the new approach @karelianpie just two final things:
🙇
- Lets move those factories into a
test-utils
folder insidesrc
so we know they are only used for testing
Done with https://github.com/yearn/yearn-sdk/pull/218/commits/3a8c1f6a20b20aefd31b2dc8d851ca993982625b
- What do you think about putting defaults in upper case so instead of
defaultAssetStaticVaultV2
we useDEFAULT_ASSET_STATIC_VAILT_V2
, just to keep consistency of constant values with best practices (https://google.github.io/styleguide/tsguide.html#identifiers)
Sorry my bad, makes total sense, as it should be, refactored with https://github.com/yearn/yearn-sdk/pull/218/commits/4fb9b905c785ff4785730994937dab700d6cbc5f
@FiboApe I think it's best practice if the reviewer resolves the comments, so that it's easier for the author of the PR to see if something was not addressed. In all review comments I'll leave a link to the commit that addresses it, then it would be great if you could double check that my commit addresses your suggestion/comment and then resolve it so I know it's all good. What do you think?
Closes https://github.com/yearn/yearn-sdk/issues/213
TokenInterface
Add a new dependency (factory.ts) for easier creation of factories;