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

Fix IQ balance not showing in stake page #258

Closed
wants to merge 1 commit into from
Closed

Conversation

SidharthK2
Copy link
Contributor

@SidharthK2 SidharthK2 commented May 3, 2024

Fix IQ balance not showing in stake page

  • Wagmi v2 deprecated useBalance for non-native tokens

image

Linked issues

closes EveripediaNetwork/issues#2608

@SidharthK2 SidharthK2 requested a review from Softdev1 May 3, 2024 10:48
Copy link

stackblitz bot commented May 3, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

vercel bot commented May 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
iq-dashboard ✅ Ready (Inspect) Visit Preview May 3, 2024 10:49am

Copy link

codeclimate bot commented May 3, 2024

Code Climate has analyzed commit b0fcadb and detected 0 issues on this pull request.

View more on Code Climate.

@kesar
Copy link
Member

kesar commented May 3, 2024

I guess lets ensure we dont migrate important things like blockchain library without testing. I can see a million of cases needed to test for such important change.

I assume nothing related to staking has been tested since the basic balance wasnt showing, so checkpoint increase amount increase time withdraw and claim hasnt being tested neither after migrating the library.

cc: @Softdev1 this is the kind of things I pointed out earlier. This got merged 2h before the call. Same issue that landing page, site broken.
cc: @Tolu-Mals if you are the reviewer you need to ensure not only code looks good, but it actually works.

@SidharthK2
Copy link
Contributor Author

I guess lets ensure we dont migrate important things like blockchain library without testing. I can see a million of cases needed to test for such important change.

I assume nothing related to staking has been tested since the basic balance wasnt showing, so checkpoint increase amount increase time withdraw and claim hasnt being tested neither after migrating the library.

cc: @Softdev1 this is the kind of things I pointed out earlier. This got merged 2h before the call. Same issue that landing page, site broken. cc: @Tolu-Mals if you are the reviewer you need to ensure not only code looks good, but it actually works.

In hindsight I think it should have been 2 separate PRs. The previous PR can be reverted and only the claim check can be implemented, wagmi upgrade can be in another PR which we can test more.

@Softdev1 Softdev1 requested a review from Tolu-Mals May 3, 2024 11:20
@Softdev1
Copy link
Contributor

Softdev1 commented May 3, 2024

I guess lets ensure we dont migrate important things like blockchain library without testing. I can see a million of cases needed to test for such important change.

I assume nothing related to staking has been tested since the basic balance wasnt showing, so checkpoint increase amount increase time withdraw and claim hasnt being tested neither after migrating the library.

cc: @Softdev1 this is the kind of things I pointed out earlier. This got merged 2h before the call. Same issue that landing page, site broken. cc: @Tolu-Mals if you are the reviewer you need to ensure not only code looks good, but it actually works.

Proper testing would be done

@Tolu-Mals
Copy link
Contributor

I guess lets ensure we dont migrate important things like blockchain library without testing. I can see a million of cases needed to test for such important change.

I assume nothing related to staking has been tested since the basic balance wasnt showing, so checkpoint increase amount increase time withdraw and claim hasnt being tested neither after migrating the library.

cc: @Softdev1 this is the kind of things I pointed out earlier. This got merged 2h before the call. Same issue that landing page, site broken. cc: @Tolu-Mals if you are the reviewer you need to ensure not only code looks good, but it actually works.

noted sir, will correct this going forward

@kesar
Copy link
Member

kesar commented May 10, 2024

this PR has conflicts

@kesar
Copy link
Member

kesar commented May 14, 2024

this PR has conflicts

@Softdev1

@SidharthK2
Copy link
Contributor Author

this PR has conflicts

@Softdev1

I think this PR can be closed, issue has been solved in #263

@kesar
Copy link
Member

kesar commented May 14, 2024

awesome 👍🏻

@kesar kesar closed this May 14, 2024
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.

Fix stake page IQ balance not showing
4 participants