-
Notifications
You must be signed in to change notification settings - Fork 156
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Switch to GitHub Actions #211
Conversation
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? |
I added a dummy workflow. Can you push to the branch to see if it builds now? |
Ah, the builds failed because it can't find the 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 coverallsapp/github-action#30. |
Do you need me to do something about this? |
Yes, you'd need to add a |
Done. |
Hmm, it still can't find it! :/ |
Maybe disable coveralls until the other tests pass? |
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)". |
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! |
Sounds good. |
The travis builds seem to have stopped working, so it would be nice to move this pull request forward :-) Is it ready for review? |
@jendrikseipp yep, this is good-to-go. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just some minor nitpicks :-) If you like, please add a changelog entry.
Thanks, Rahul! Very nice that all tests are consolidated now! |
Yay! Here's a list of things I'd like to tackle next:
|
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! |
Yep, yep! Run Vulture, match the output against a regex and annotate the troubling lines. All credit to @The-Compiler for the idea! :D |
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? |
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) |
Sounds good. Looking forward to seeing what you cook up :-) |
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/settingsChecklist: