Page MenuHomePhabricator

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

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

Details

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
Branch
PR12284
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 7089
Build 12224: Bitcoin ABC Buildbot (legacy)
Build 12223: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Aug 7 2019, 00:11
Fabien requested changes to this revision.Aug 7 2019, 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.Aug 7 2019, 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..Aug 7 2019, 18:06
nakihito edited the summary of this revision. (Show Details)
nakihito requested review of this revision.Aug 7 2019, 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 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..Aug 7 2019, 21:03
nakihito edited the summary of this revision. (Show Details)
nakihito edited the test plan for this revision. (Show Details)
nakihito requested review of this revision.Aug 7 2019, 23:22

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

Fabien requested changes to this revision.Aug 8 2019, 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.Aug 8 2019, 08:38

Removed non-functional lines and no longer need imports.

This revision is now accepted and ready to land.Aug 8 2019, 20:49