Page MenuHomePhabricator

[Cashtab] Remove unused state param in Config
ClosedPublic

Authored by bytesofman on Feb 17 2024, 23:31.

Details

Reviewers
emack
Group Reviewers
Restricted Project
Commits
rABC37c413c5f950: [Cashtab] Remove unused state param in Config
Summary

This is used on the OnBoarding screen to prevent validation from happening while a user enters a mnemonic, since it would just fail until it is completely entered. The implementation in OnBoarding is itself a bit hard to follow and could be improved.

But it's not doing anything here and should just be removed. Probably copypasta from its use on OnBoarding.

Test Plan

npm test
git grep dirty and it's only in OnBoarding.js

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bytesofman edited the test plan for this revision. (Show Details)
emack requested changes to this revision.Feb 18 2024, 09:13
emack added a subscriber: emack.

I ran npm test 5 times on this diff and 5 times on master. This diff was getting consistent jest timeouts over Configure.js whilst master was all ok.
Normally I'd chalk timeouts to local factors but this seems rather specific to this dirty param you're removing.
Is this the case for you?

image.png (126×564 px, 17 KB)

This revision now requires changes to proceed.Feb 18 2024, 09:13

nvm, started observing the timeouts with other diffs so not for resolution here. Just needs a separate timeout bump diff.

This revision is now accepted and ready to land.Feb 18 2024, 10:18

I ran npm test 5 times on this diff and 5 times on master. This diff was getting consistent jest timeouts over Configure.js whilst master was all ok.
Normally I'd chalk timeouts to local factors but this seems rather specific to this dirty param you're removing.
Is this the case for you?

image.png (126×564 px, 17 KB)

The single test in Configure.test.js is quite long now, since the multiple tests that repeated a lot of the initial setup tests were combined into one. I haven't been hitting this timeout locally yet but it is getting close. Will bump in a separate diff.