Page MenuHomePhabricator

Merge #12284: Remove assigned but never used local variables. Enable linter checking for unused local variables.
ClosedPublic

Authored by nakihito on Wed, Aug 7, 00:11.

Details

Reviewers
deadalnix
Fabien
jasonbcox
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABC4afd5d82e98d: Merge #12284: Remove assigned but never used local variables. Enable linter…
Summary

ea04bf7862 Enable flake8 warning F841 ("local variable 'foo' is assigned to but never used") (practicalswift)
169f3e8637 Remove assigned but never used local variables (practicalswift)

Pull request description:

Remove assigned but never used local variables. Enable Travis checking for unused local variables.

Tree-SHA512: d6052ec9044c5d1f03d874ea3c8addd5a156779213ef9200f89d3ae53230f2fd1691aff405c3dae14178e5ef09912c4432e92f606ef4a5220ed9daa140cdee81

Backport of Core PR12284
https://github.com/bitcoin/bitcoin/pull/12284/

Note: rather than Travis checking for unused local variables, our linter will check for unused local variables.

Test Plan
test_runner.py --extended
arc lint --everything

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nakihito created this revision.Wed, Aug 7, 00:11
Owners added a reviewer: Restricted Owners Package.Wed, Aug 7, 00:11
Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Aug 7, 00:11
Fabien requested changes to this revision.Wed, Aug 7, 08:38

Half of the backport is missing, the title and description don't match the content and the once the backport gets completed the test plan would need an update as well.

This revision now requires changes to proceed.Wed, Aug 7, 08:38
nakihito retitled this revision from Merge #12284: Remove assigned but never used local variables. Enable Travis checking for unused local variables. to Merge #12284: Remove assigned but never used local variables..Wed, Aug 7, 18:06
nakihito edited the summary of this revision. (Show Details)
nakihito requested review of this revision.Wed, Aug 7, 18:10

github-merge.py changes ignored because they are irrelevant to us.
p2p_segwit.py changes also ignored because we don't have/use segwit.

nakihito planned changes to this revision.Wed, Aug 7, 19:45
nakihito retitled this revision from Merge #12284: Remove assigned but never used local variables. to Merge #12284: Remove assigned but never used local variables. Enable linter checking for unused local variables..Wed, Aug 7, 21:03
nakihito edited the summary of this revision. (Show Details)
nakihito edited the test plan for this revision. (Show Details)
nakihito updated this revision to Diff 10661.Wed, Aug 7, 21:06

Added changes to linter.

nakihito planned changes to this revision.Wed, Aug 7, 21:06
nakihito requested review of this revision.Wed, Aug 7, 23:22
nakihito added a comment.Wed, Aug 7, 23:28

I also went ahead and removed the unused variables found by the linter after the change.

Fabien requested changes to this revision.Thu, Aug 8, 08:38

Some of the tests modified by this diff have their estimated duration > 40s (you can check this from test/functional/timing.json).
Therefore they are not run as part of test_runner.py but are part of the extended tests, requiring the use of test_runner.py --extended to be run.
You need to try it and add it to the test plan once you're sure that the patch did not introduce any regression.

test/functional/abc-mempool-accept-txn.py
197 ↗(On Diff #10661)

This was an unused shortcut for a function name, the statement by itself does nothing and can be removed.

test/functional/wallet_abandonconflict.py
37 ↗(On Diff #10661)

The code was building a transaction object from its txid, then did not use this object.
As there is no need for this transaction object, you can avoid creating it and remove the whole statement.

test/functional/wallet_hd.py
21 ↗(On Diff #10661)

This does nothing, remove.

This revision now requires changes to proceed.Thu, Aug 8, 08:38
nakihito updated this revision to Diff 10684.Thu, Aug 8, 20:34

Removed non-functional lines and no longer need imports.

nakihito edited the test plan for this revision. (Show Details)Thu, Aug 8, 20:34
Fabien accepted this revision.Thu, Aug 8, 20:49
This revision is now accepted and ready to land.Thu, Aug 8, 20:49