Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: financial-statement from origin-defi #125

Merged
merged 14 commits into from
Oct 6, 2023

Conversation

apexearth
Copy link
Contributor

@apexearth apexearth requested a review from nick September 29, 2023 00:25
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-em7dvw September 29, 2023 00:25 Inactive
@apexearth apexearth force-pushed the feat/financial-statement branch from 9fd56cb to e5c6af4 Compare September 29, 2023 23:34
@rafaelugolini rafaelugolini had a problem deploying to preview-oeth-feat-finan-em7dvw September 29, 2023 23:34 Failure
@rafaelugolini rafaelugolini had a problem deploying to preview-oeth-feat-finan-em7dvw September 30, 2023 01:08 Failure
@rafaelugolini rafaelugolini had a problem deploying to preview-oeth-feat-finan-em7dvw September 30, 2023 01:12 Failure
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-em7dvw September 30, 2023 14:28 Inactive
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-em7dvw October 2, 2023 16:53 Inactive
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-em7dvw October 2, 2023 19:04 Inactive
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-em7dvw October 2, 2023 21:02 Inactive
@apexearth apexearth closed this Oct 2, 2023
@apexearth apexearth deleted the feat/financial-statement branch October 2, 2023 23:46
@apexearth apexearth restored the feat/financial-statement branch October 2, 2023 23:46
@apexearth apexearth reopened this Oct 2, 2023
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-jt2oph October 2, 2023 23:47 Inactive
- change title
- loading spinner
- per-column rate conversions
- hide 'PROTOCOL NET VALUE' when negative o_0
- column alignment
- remove hover bold
- set arrow gray
- 0 decimal usd prices, unless compacted
- 3 decimal eth prices, unless compacted
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-jt2oph October 3, 2023 21:10 Inactive
- reverse column order, showing now first
- change column header formatting & refine dates shown
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-jt2oph October 3, 2023 21:35 Inactive
- tweak negative color
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-jt2oph October 3, 2023 21:45 Inactive
# Conflicts:
#	package.json
#	yarn.lock
@rafaelugolini rafaelugolini had a problem deploying to preview-oeth-feat-finan-jt2oph October 3, 2023 23:19 Failure
@apexearth apexearth requested a review from toniocodo October 4, 2023 17:22
Copy link
Contributor

@toniocodo toniocodo left a comment

Choose a reason for hiding this comment

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

It overall LGTM, just had a couple remarks regarding migrating to wagmi v1+ and viem. Same with react-intl, might not be worthy if only a couple of labels are using it

"wagmi": "^0.12.7"
"usehooks-ts": "^2.9.1",
"viem": "^1.10.12",
"wagmi": "^1.4.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

wagmi v1 introduced quite a lot of changes, are you sure to bump version? In case of, they still maintain 0.12.x 😓

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't used in many places and I made the changes to accommodate the new bigint types returned.

@@ -40,9 +42,13 @@
"sanitize-html": "^2.10.0",
"tailwind-merge": "^1.12.0",
"tailwindcss": "^3.3.1",
"wagmi": "^0.12.7"
"usehooks-ts": "^2.9.1",
"viem": "^1.10.12",
Copy link
Contributor

Choose a reason for hiding this comment

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

this is replacement for ethers.js, quite a lot of breaking changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes though we already had wagmi in the project. What I did here is upgrade wagmi and it was used in very few places.

package.json Outdated Show resolved Hide resolved
- remove `react-intl`
@rafaelugolini rafaelugolini temporarily deployed to preview-oeth-feat-finan-jt2oph October 4, 2023 19:13 Inactive
- add balancer aura strategy
- remove some useless generated code
@apexearth apexearth merged commit 7d6469d into master Oct 6, 2023
@nick nick deleted the feat/financial-statement branch April 26, 2024 20:21
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.

3 participants