[Cashtab] Remove unused state param in Config
ClosedPublic
Actions

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

Branch

remove-unused-dirty

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27223
Build 54010: Build Diff	cashtab-tests
Build 54009: arc lint + arc unit

Event Timeline

bytesofman created this revision.Feb 17 2024, 23:31

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 17 2024, 23:31

bytesofman edited the summary of this revision. (Show Details)Feb 17 2024, 23:32

bytesofman edited the test plan for this revision. (Show Details)

Harbormaster completed remote builds in B27223: Diff 45350.Feb 17 2024, 23:34

bytesofman published this revision for review.Feb 17 2024, 23:35

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?

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

In D15475#348774, @emack wrote:

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?

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.

Closed by commit rABC37c413c5f950: [Cashtab] Remove unused state param in Config (authored by bytesofman). · Explain WhyFeb 18 2024, 13:36

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABC37c413c5f950: [Cashtab] Remove unused state param in Config.