Skip to content

test: Remove vulture from ci, Remove some --min-confidence=60 unused code#34816

Open
maflcko wants to merge 6 commits intobitcoin:masterfrom
maflcko:2603-ci-lint-less
Open

test: Remove vulture from ci, Remove some --min-confidence=60 unused code#34816
maflcko wants to merge 6 commits intobitcoin:masterfrom
maflcko:2603-ci-lint-less

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Mar 12, 2026

Currently, vulture is run with --min-confidence=100, which reduces its checks to dead code after control statements, which is nice, but not really a common nor severe issue. See the discussion in #34810 (comment) and commit 5c00536, which had to remove dead code manually.

Reducing the confidence has shown to be too brittle/tedious in the past, so remove the tool for now from CI.

Of course, removing this from CI does not prevent anyone from running it locally and removing dead code.

Fixes #34810

@DrahtBot DrahtBot added the Tests label Mar 12, 2026
@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 12, 2026 •
edited
Loading

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK willcl-ark
Stale ACK l0rinc

If your review is incorrectly listed, please copy-paste <!--meta-tag:bot-skip--> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #34547 (lint: modernise lint tooling by willcl-ark)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko maflcko mentioned this pull request Mar 12, 2026
Open
@l0rinc
Copy link
Contributor

l0rinc commented Mar 12, 2026

code review ACK fab5072

@maflcko maflcko changed the title ci: Remove vulture test: Remove vulture from ci, Remove some --min-confidence=60 unused code Mar 12, 2026
@maflcko
Copy link
Member Author

maflcko commented Mar 12, 2026

Of course, removing this from CI does not prevent anyone from running it locally and removing dead code.

Went ahead and did this myself. I ran vulture $( git ls-files -- '*.py' ) and then manually went through the ~200 lines and removed the stuff that was really unused.

@willcl-ark
Copy link
Member

willcl-ark commented Mar 12, 2026

I see this locally?

F401 [*] `test_framework.script_util.key_to_p2pk_script` imported but unused
  --> test/functional/feature_segwit.py:32:5
   |
30 | )
31 | from test_framework.script_util import (
32 |     key_to_p2pk_script,
   |     ^^^^^^^^^^^^^^^^^^
33 |     key_to_p2wpkh_script,
34 |     keys_to_multisig_script,
   |
help: Remove unused import

F401 [*] `test_framework.script_util.key_to_p2wpkh_script` imported but unused
  --> test/functional/feature_segwit.py:33:5
   |
31 | from test_framework.script_util import (
32 |     key_to_p2pk_script,
33 |     key_to_p2wpkh_script,
   |     ^^^^^^^^^^^^^^^^^^^^
34 |     keys_to_multisig_script,
35 |     script_to_p2sh_script,
   |
help: Remove unused import

F401 [*] `test_framework.script_util.script_to_p2sh_script` imported but unused
  --> test/functional/feature_segwit.py:35:5
   |
33 |     key_to_p2wpkh_script,
34 |     keys_to_multisig_script,
35 |     script_to_p2sh_script,
   |     ^^^^^^^^^^^^^^^^^^^^^
36 |     script_to_p2wsh_script,
37 | )
   |
help: Remove unused import

F401 [*] `test_framework.script_util.script_to_p2wsh_script` imported but unused
  --> test/functional/feature_segwit.py:36:5
   |
34 |     keys_to_multisig_script,
35 |     script_to_p2sh_script,
36 |     script_to_p2wsh_script,
   |     ^^^^^^^^^^^^^^^^^^^^^^
37 | )
38 | from test_framework.test_framework import BitcoinTestFramework
   |
help: Remove unused import

F401 [*] `enum` imported but unused
  --> test/functional/test_framework/address.py:11:8
   |
 9 | - bech32m segwit v1 P2TR addresses."""
10 |
11 | import enum
   |        ^^^^
12 | import unittest
   |
help: Remove unused import: `enum`

Found 5 errors.
[*] 5 fixable with the `--fix` option.
^^^
`ruff` found errors!
^---- ⚠️ Failure generated from lint check 'py_lint' (Lint Python code)!


src/crc32c in HEAD currently refers to tree 28f07d0bd5d6fcd3273d9651ff3c960ad344c202
src/crc32c in HEAD was last updated in commit 9a5d29711afcdc4609da4786673758e641958bb4 (tree 28f07d0bd5d6fcd3273d9651ff3c960ad344c202)
GOOD
src/crypto/ctaes in HEAD currently refers to tree 1b6c31139a71f80245c09597c343936a8e41d021
src/crypto/ctaes in HEAD was last updated in commit 8501bedd7508ac514385806e191aec21ee978891 (tree 1b6c31139a71f80245c09597c343936a8e41d021)
GOOD
src/ipc/libmultiprocess in HEAD currently refers to tree f88433d756e02de05276361a5c7e41c71dd58231
src/ipc/libmultiprocess in HEAD was last updated in commit b7ca3bf061b51108d155283c1ad503c0af7eab0d (tree f88433d756e02de05276361a5c7e41c71dd58231)
GOOD
src/leveldb in HEAD currently refers to tree fe97d24767d36e3d80902af8da83a46f68c34b96
src/leveldb in HEAD was last updated in commit 2f2952c5f2e367ccac34025a159c9fbaffd7f172 (tree fe97d24767d36e3d80902af8da83a46f68c34b96)
GOOD
src/minisketch in HEAD currently refers to tree f1581edd423bcf531d0d956149eb8abe45f75570
src/minisketch in HEAD was last updated in commit 4543a3bde26ff2440c16b06cc1dcf1994dc85720 (tree f1581edd423bcf531d0d956149eb8abe45f75570)
GOOD
src/secp256k1 in HEAD currently refers to tree 15001ae05f06101b13596633fad8c75dcec693b0
src/secp256k1 in HEAD was last updated in commit 9d4c9b00356e02de223033adb7680b47b743caab (tree 15001ae05f06101b13596633fad8c75dcec693b0)
GOOD
test/functional/feature_segwit.py:70: error: Need type annotation for "txs_mined" (hint: "txs_mined: dict[<type>, <type>] = ...")  [var-annotated]
Found 1 error in 1 file (checked 311 source files)
^---- ⚠️ Failure generated from lint-python.py
^^^

^---- ⚠️ Failure generated from lint check 'all_python_linters' (Run all linters of the form: test/lint/lint-*.py)!

@maflcko maflcko force-pushed the 2603-ci-lint-less branch from fa15057 to fa90b21 Compare March 12, 2026 15:01
@maflcko
Copy link
Member Author

maflcko commented Mar 12, 2026

I see this locally?

Thx, removed all those lines as well. Let's see if lint CI passes ... edit: passed

willcl-ark
willcl-ark approved these changes Mar 14, 2026
View reviewed changes
Copy link
Member

@willcl-ark willcl-ark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK fa90b21

Changes LGTM.

Of course, removing this from CI does not prevent anyone from running it locally and removing dead code.

Well, its free to run on GHA so stuck it in my nightly repo at confidence level 100.

Could probably set it to run monthly at confidence=60 and have claude prune the output for false positives, but not sure there is enough demand/benefit.

@DrahtBot DrahtBot requested a review from l0rinc March 14, 2026 10:30
@DrahtBot DrahtBot mentioned this pull request Mar 14, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@l0rinc l0rinc Awaiting requested review from l0rinc

1 more reviewer

@willcl-ark willcl-ark willcl-ark approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test/lint/lint-python-dead-code false positives

4 participants

@maflcko @DrahtBot @l0rinc @willcl-ark