602 add vulture

[VFermat]

Fixes #602

Adds a GitHub action to run Vulture to Pushes and Pull Requests.
Vulture looks for unused code.

Technical details
Vulture gives confidence on the unused code found. For this action I went with 90%, as used on this action

Checklist

• My pull request has a descriptive title (not a vague title like Update index.md).
• My pull request targets the master branch of the repository
• My commit messages follow best practices.
• My code follows the established code style of the repository.
• I added tests for the changes I made (if applicable).
• I added or updated documentation (if applicable).
• I tried running the project locally and verified that there are no
  visible errors.

Developer Certificate of Origin

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

Vitor Eller

[seancolsen]

Thanks for this PR, @VFermat!

@kgodey or @mathemancer would you care to review this?

[kgodey]

@seancolsen I'll take it, I've assigned myself. Thanks!

[kgodey]

@VFermat This looks great, but the output doesn't show which file the dead code is in so it's hard to fix. See: https://github.com/centerofci/mathesar/runs/4142244656?check_suite_focus=true

This seems to be an issue with the action. I'm not sure that we need to use a GitHub action specific to Vulture, perhaps just running Vulture manually would work? e.g. see this example of a workflow where we run a Python script on the code. https://github.com/centerofci/mathesar-wiki/blob/master/.github/workflows/run-link-rot-detection.yml

[VFermat]

@kgodey alright! I will test it as a workflow!

[VFermat]

@kgodey adding a reminder here!

[kgodey]

@VFermat sorry, I didn't realize this was ready for re-review, the usual practice is to request re-review via GitHub (step 7 here: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review)

[VFermat]

@kgodey sorry for that!

I don't think I have permission to alter the status of my pull requests tho... Can you check that for me please?

[kgodey]

@VFermat Can you click this button in the top right of the PR?

[kgodey]

I don't see any output here when vulture runs: https://github.com/centerofci/mathesar/runs/4230565151?check_suite_focus=true

Running vulture . --min-confidence 90 locally results in this:

/Users/kgodey/Code/mathesar/db/types/email.py:26: unused variable 'kw' (100% confidence)
/Users/kgodey/Code/mathesar/db/types/money.py:16: unused variable 'kw' (100% confidence)
/Users/kgodey/Code/mathesar/db/types/uri.py:45: unused variable 'kw' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/migrations/0012_transfer_database.py:6: unused variable 'schema_editor' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/migrations/0012_transfer_database.py:15: unused variable 'schema_editor' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/signals.py:9: unused variable 'sender' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/tests/api/test_schema_api.py:23: unused variable 'empty_nasa_table' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/tests/conftest.py:46: unused variable 'django_db_setup' (100% confidence)
/Users/kgodey/Code/mathesar/mathesar/tests/test_multi_db.py:71: unused variable 'multi_db_test_db' (100% confidence)

I don't see that on the action output.

[seancolsen]

@VFermat I'm looking over our open PRs and noticed that this one still need some changes from you to resolve @kgodey's feedback above. It would be great to get this merged. Are you still working on it? Do you need any help or clarification on the review? Thanks!

[mathemancer]

@kgodey All the things found in your local run appear to be specifically excluded by the lines in pyproject.toml. At least one of those things makes sense to exclude (kw is idiomatic where it's used). Should we just start with those excluded for now, or would you rather start with nothing excluded? I prefer the latter, so I'll set it up to do that, and continue on error.

[kgodey]

@mathemancer I apparently didn't think to check the configuration, thanks for pointing that out.

I think we should exclude the things we don't intend to fix (including skipping the migrations directories entirely) and remove the continue on error. The idea behind setting this up was to make sure we deleted unused code in PRs.

[mathemancer]

Closing this PR, since it's a bit tedious to work on the fork without permissions to that repo, so I'm creating a branch in this repo, and basing a new PR #965 off of that. @VFermat If you're out there, thank you very much for your work on this. @kgodey Please see the new PR for continued review / discussion.