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

refactor: implement my votes page design changes #618

Merged
merged 25 commits into from
Aug 22, 2024

Conversation

shahin-hq
Copy link
Contributor

@shahin-hq shahin-hq commented Aug 16, 2024

Summary

Closes: https://app.clickup.com/t/86du08j9r

Checklist

  • My changes look good in both light AND dark mode
  • The change is not hardcoded to a single network, but has multi-asset in mind
  • I checked my changes for obvious issues, debug statements and commented code
  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

Copy link

vercel bot commented Aug 16, 2024

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

Name Status Preview Updated (UTC)
arkvault ✅ Ready (Inspect) Visit Preview Aug 21, 2024 11:08am

COVERAGE_THRESHOLD_BRANCHES: 98.48
COVERAGE_THRESHOLD_LINES: 98.67
COVERAGE_THRESHOLD_STATEMENTS: 98.73
COVERAGE_THRESHOLD_BRANCHES: 98.35
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't really tell why it was failing due to coverage as it has matching values with a passing test:

Passing test from develop: https://github.com/ArdentHQ/arkvault/actions/runs/10486923172/job/29046217452

Failing test: https://github.com/ArdentHQ/arkvault/actions/runs/10486395863/job/29044533608

Copy link
Contributor

Choose a reason for hiding this comment

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

the reason is that the thresholds depend on a variety of files. Now if you end up refactoring areas in these files, it's possible that the total amount of branches goes down for example. If you don't change the tests (other than removing the ones that are obsolete) it's possible that the coverage goes down because of the amount of branches remaining. Example:

  • you have 11 lines of code, 10 are covered = 90.9% coverage
  • you remove 1 covered line of code: 10 lines of code remaining, 9 are covered = 90% coverage

this doesn't mean that the code in itself has become less covered, as the exact same assertions are still being made to before, but because it's percentage based it does drop in overall coverage. This will always be a thing; only when we'd have 100% coverage everywhere would this not happen

<tr key={key} {...headerGroupProps}>
{headerGroup.headers.map(renderColumn)}
</tr>
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to fix warning:

image

);
}

// @TODO handle multiple validators
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ItsANameToo ItsANameToo merged commit 843695f into develop Aug 22, 2024
29 checks passed
@ItsANameToo ItsANameToo deleted the refactor/implement-my-votes-page-changes branch August 22, 2024 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants