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

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

Details

Reviewers

deadalnix
Fabien
jasonbcox

Group Reviewers

Restricted Owners Package	(Owns No Changed Paths)
Restricted Project

Commits

rSTAGING4afd5d82e98d: Merge #12284: Remove assigned but never used local variables. Enable linter…
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

Branch

PR12284

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7075
Build 12196: Bitcoin ABC Buildbot (legacy)
Build 12195: arc lint + arc unit

Event Timeline

nakihito created this revision.Aug 7 2019, 00:11

Owners added a reviewer: Restricted Owners Package.Aug 7 2019, 00:11

Herald added a reviewer: Restricted Project. · View Herald TranscriptAug 7 2019, 00:11

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7064: Diff 10648.Aug 7 2019, 00:17

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)

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.Aug 7 2019, 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..Aug 7 2019, 21:03

nakihito edited the summary of this revision. (Show Details)

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

Added changes to linter.

nakihito planned changes to this revision.Aug 7 2019, 21:06

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7075: Diff 10661.Aug 7 2019, 21:17

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.

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	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	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	This does nothing, remove.