Skip to content

VelodromeExchangeAdapter#257

Open
andris-balodis wants to merge 10 commits intoSetProtocol:masterfrom
andris-balodis:velodrome-adapter
Open

VelodromeExchangeAdapter#257
andris-balodis wants to merge 10 commits intoSetProtocol:masterfrom
andris-balodis:velodrome-adapter

Conversation

@andris-balodis
Copy link
Copy Markdown

Comment thread test/integration/velodromeExchangeTradeModule.spec.ts
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts Outdated
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts Outdated
Comment thread test/integration/velodromeExchangeTradeModule.spec.ts
@FlattestWhite
Copy link
Copy Markdown
Contributor

Also looks like the integration tests are failing on CI

Comment thread contracts/interfaces/external/IVelodromeRouter.sol Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
Comment thread contracts/protocol/integration/exchange/VelodromeExchangeAdapter.sol Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts
Copy link
Copy Markdown
Contributor

@FlattestWhite FlattestWhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly looks pretty good - I'm just wondering with all the extra interface objects, where they are needed. I was under the impression only the abi json files are required

Comment thread utils/deploys/deployAdapters.ts Outdated
Comment thread test/integration/velodromeExchangeTradeModule.spec.draft.ts
Comment thread utils/deploys/deployExternal.ts Outdated
Comment thread external/contracts/velodrome/VelodromePair.sol Outdated
Comment thread external/contracts/velodrome/interfaces/IERC20.sol Outdated
Comment thread external/contracts/velodrome/lib/Math.sol Outdated
Comment thread test/integration/velodromeExchangeTradeModule.spec.draft.ts
Copy link
Copy Markdown
Contributor

@FlattestWhite FlattestWhite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nearly there - a few more comments on the style of the unit test to be consistent with set's expected style

Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
Comment thread utils/test/index.ts Outdated
Comment thread utils/fixtures/velodromeFixture.ts Outdated
Comment thread utils/contracts/velodrome.ts Outdated
Comment thread test/protocol/integration/exchange/velodromeExchangeAdapter.spec.ts Outdated
@andris-balodis andris-balodis marked this pull request as ready for review June 30, 2022 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants