-
Notifications
You must be signed in to change notification settings - Fork 8
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Pull Request Test Coverage Report for Build 4363399208
💛 - Coveralls |
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.
Hey @shoom3301 , great job!
But I have found 1 issue that blocks my approval: AppData details is not working in your PR
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.
I'm concerned with the subgraph env var urls.
See the comment
src/apps/explorer/components/SummaryCardsWidget/VolumeChart/useGetVolumeData.ts
Show resolved
Hide resolved
@elena-zh thanks! Fixed |
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.
Hey @shoom3301 , I'm approving the current PR. However, I found a list of new issues I faced for the updated AppData page, and reported them in #392. It would be great to fix them.
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.
👍
SDK refactoring: cowprotocol/cow-sdk#97
Changes
optimize-bundle
script, we don't need it since we got rid of redundant dependenciesSupportedChainId
,OrderKind
,OrderClass
importsmetadataApiSDK
usageOrderBookApi
Gas price strategy
code0x
andMatcha
APIs due to their irrelevancegetNativePrice
by import from SDKsendOrder
andsendSignedOrderCancellation
form SDK@cowprotocol/contracts
, because they are pretty heavy and we don't need them most of the timeOther PRs: https://github.com/cowprotocol/cowswap/pulls?q=is%3Apr+is%3Aclosed+label%3ASDK_dogfooding