Skip to content

Conversation

@naevern
Copy link

@naevern naevern commented May 22, 2025

This PR lowers the minimum confidence threshold for Vulture from 100 to 60, allowing it to detect more potentially unused code. This change will help identify dead code that might otherwise be missed with the stricter setting.

Fixes #1239

@SamWilsn
Copy link
Contributor

Presumably this'll pick up new errors when you run tox? Unfortunately our CI is broken at the moment, but you can run it locally. Part of closing this issue would be fixing/ignoring any diagnostics that vulture reports.

Unless you have run it locally and it's clean?

@naevern naevern changed the title lower vulture min confidence from 100 to 60 lower vulture min confidence Jun 2, 2025
@naevern
Copy link
Author

naevern commented Jun 2, 2025

Hi @SamWilsn @gurukamath ,
after revewing tox.ini again, I've run the full test suite locally and can confirm the findings:

running tox in terminal with:

  • confidence 80: no new vulture warnings (clean run)
  • confidence 60: more than 100 unused code warnings (also causes static analysis failure)

I've updated the PR to use confidence 80 as it provides a good balance of lowering the threshold from 100 while avoiding the introduction of many warnings that would need individual review. this approach gives us incremental progress toward catching more dead code while maintaining a clean build. can you have a review on it now if everything looks good to go?

@Carsons-Eels
Copy link
Contributor

I experimented and we can actually lower the threshold all the way down to 61 and still get a clean run since all issues it detects right now trigger at 60.

@naevern
Copy link
Author

naevern commented Jun 2, 2025

thank for the preferred value @Carsons-Eels . so, I'm settings the value to 61 and doing commit on this PR again.

@naevern
Copy link
Author

naevern commented Jun 14, 2025

@Carsons-Eels any update on the change if it's good to go?

@Carsons-Eels
Copy link
Contributor

Hi @naevern, we had a co-working team meetup last week where we discussed how to approach dead code detection, and decided that there were few enough issues to resolve or whitelist that we are going to not limit vulture's detection threshold and instead maintain the highest level of cleanliness in this repo that we can.

We won't be moving forward with this PR as a result, but thank you for your contribution.

Carsons-Eels pushed a commit to Carsons-Eels/execution-specs that referenced this pull request Oct 16, 2025
…sts subfolder (ethereum#1241)

Co-authored-by: danceratopz <danceratopz@gmail.com>
danceratopz added a commit to danceratopz/execution-specs that referenced this pull request Oct 22, 2025
…sts subfolder (ethereum#1241)

Co-authored-by: danceratopz <danceratopz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lower the vulture min confidence

3 participants