jendrikseipp / vulture

Find dead Python code
MIT License
3.42k stars 150 forks source link

Switch to GitHub Actions #211

Closed RJ722 closed 4 years ago

RJ722 commented 4 years ago

Description

Switch to GitHub Actions

@jendrikseipp, in order to report to coveralls, you'd need to add a secret COVERALLS_REPO_TOKEN. The repo-token can be found here: https://coveralls.io/github/jendrikseipp/vulture/settings

Checklist:

RJ722 commented 4 years ago

https://github.community/t/run-a-github-action-on-pull-request-for-pr-opened-from-a-forked-repo/16054/2

Seems like inter-fork PRs introducing workflows don't trigger a build. @jendrikseipp can you pull this branch from my fork, and push as one of your branch, please?

jendrikseipp commented 4 years ago

I added a dummy workflow. Can you push to the branch to see if it builds now?

RJ722 commented 4 years ago

Ah, the builds failed because it can't find the COVERALLS_REPO_TOKEN.

Feedback for @coverallsapp: It would be really nice to have pytest-coverage report supported for your GitHub Action. I am very unpleased with a lack of response from your end at https://github.com/coverallsapp/github-action/issues/30.

jendrikseipp commented 4 years ago

Ah, the builds failed because it can't find the COVERALLS_REPO_TOKEN.

Do you need me to do something about this?

RJ722 commented 4 years ago

Do you need me to do something about this?

Yes, you'd need to add a COVERALLS_REPO_TOKEN secret in GitHub (Repo Settings -> Secrets). The repo-token can be found here: https://coveralls.io/github/jendrikseipp/vulture/settings.

jendrikseipp commented 4 years ago

Done.

RJ722 commented 4 years ago

Hmm, it still can't find it! :/

jendrikseipp commented 4 years ago

Maybe disable coveralls until the other tests pass?

jendrikseipp commented 4 years ago

The settings say "Secrets are not passed to workflows that are triggered by a pull request from a fork. (https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets)".

jendrikseipp commented 4 years ago

Maybe we need to use the coveralls GitHub action? https://github.com/marketplace/actions/coveralls-github-action

RJ722 commented 4 years ago

Maybe we need to use the coveralls GitHub action? https://github.com/marketplace/actions/coveralls-github-action

But, that doesn't support pytest-cov yet! We don't know whether they plan to support it at all, or not.

I'll remove the coveralls integration for now. We can add later!

jendrikseipp commented 4 years ago

I'll remove the coveralls integration for now. We can add later!

Sounds good.

jendrikseipp commented 4 years ago

The travis builds seem to have stopped working, so it would be nice to move this pull request forward :-) Is it ready for review?

RJ722 commented 4 years ago

@jendrikseipp yep, this is good-to-go.

jendrikseipp commented 4 years ago

Thanks, Rahul! Very nice that all tests are consolidated now!

RJ722 commented 4 years ago

Yay! Here's a list of things I'd like to tackle next:

jendrikseipp commented 4 years ago

Nice! All three sound very good!

Can you elaborate on the third one? Are you planning to write a GitHub Action that anyone can add to their repositories to run Vulture? That sounds cool!

RJ722 commented 4 years ago

Can you elaborate on the third one? Are you planning to write a GitHub Action that anyone can add to their repositories to run Vulture?

Yep, yep! Run Vulture, match the output against a regex and annotate the troubling lines.

All credit to @The-Compiler for the idea! :D

jendrikseipp commented 4 years ago

Nice! I guess you plan to use regexes because we can't use the API? Can't the Action use Python and use the API?

The-Compiler commented 4 years ago

It's certainly possible to use the GitHub API to submit annotations, but registering a problem matcher file like:

{
  "problemMatcher": [
    {
      "severity": "warning",
      "pattern": [
        {
          "regexp": "^([^:]+):(\\d+): ([^(]+ \\(\\d+% confidence\\))$",
          "file": 1,
          "line": 2,
          "message": 3
        }
      ],
      "owner": "vulture"
    }
  ]
}

Seems like a much simpler solution (and doesn't require an API key and such)

jendrikseipp commented 4 years ago

Sounds good. Looking forward to seeing what you cook up :-)