Skip to content
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

Add Github Action definition #11320

Merged
merged 3 commits into from
Oct 19, 2021
Merged

Add Github Action definition #11320

merged 3 commits into from
Oct 19, 2021

Conversation

elprans
Copy link
Contributor

@elprans elprans commented Oct 12, 2021

Define a Github Action that can be used as a canonical way of running mypy as part of Github CI workflows as easy as:

jobs:
  lint:
    runs-on: ubuntu-latest
    steps:
      - uses: python/mypy@master

The action is inspired by its equivalent in black and supports the following options:

with:
   options: "..."  # mypy command-line options
   paths: "."  # a list of paths to check
   version: "0.910"  # mypy version to use
   install_types: yes|no  # whether to auto-install stubs

Example run

Define a Github Action that can be used as a canonical way of running
mypy as part of Github CI workflows as easy as:

    jobs:
      lint:
        runs-on: ubuntu-latest
        steps:
          - uses: python/mypy

The action is inspired by its equivalent in black and supports the
following options:

    with:
       options: "..."  # mypy command-line options
       paths: "."  # a list of paths to check
       version: "0.910"  # mypy version to use
       install_types: yes|no  # whether to auto-install stubs
* Default to argument-less invocation instead of `.` so `files` in config works
* Add `install_project_dependencies` (defaults to "yes"), which installs
  project dependencies into mypy's virtualenv (needed for PEP 561 deps).
Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

This looks pretty reasonable, and could be pretty useful for people. My main question is where the best location to put it is. Does anyone have opinions on this?

@hauntsaninja
Copy link
Collaborator

Thanks, this does seem like it could be useful.

I think it has to be at the root of the repo. Frankly, I wouldn't mind having this live in another repo (similar to how pyright does it https://github.com/jakebailey/pyright-action), the wide variety of project installations and configurations people have might make this an annoying thing to maintain. Speaking of the pyright-action, it does some cool Github magic to have errors show up in your PR, which is pretty nice

@elprans
Copy link
Contributor Author

elprans commented Oct 14, 2021

I think it has to be at the root of the repo

Not necessarily, it can be in a subfolder, e.g. action, in which case you'd refer to the action as python/mypy/action@master. That said, most users are probably unaware of this, since the vast majority of actions are just org/repo.

it does some cool Github magic to have errors show up in your PR, which is pretty nice

We can add a problem matcher which would convert mypy errors into Github annotations via regexps.

Copy link
Collaborator

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

I think its fine to keep it at the toplevel. It also would be awesome to add problem matchers, the annotations in the Github UI are pretty nice.

@JukkaL
Copy link
Collaborator

JukkaL commented Oct 19, 2021

Thanks!

@JukkaL JukkaL merged commit e028045 into python:master Oct 19, 2021
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 19, 2021

Added #11359 to track documentation for this.

hauntsaninja pushed a commit to hauntsaninja/mypy that referenced this pull request Oct 31, 2021
This was broken by python#11320
@hauntsaninja hauntsaninja mentioned this pull request Oct 31, 2021
JelleZijlstra pushed a commit that referenced this pull request Oct 31, 2021
This was broken by #11320

Co-authored-by: hauntsaninja <>
@sobolevn
Copy link
Member

I am working on #11359, I have some feedback on currect state of mypy's GitHub Action:

  1. It is not publushed. It might not be a problem at a first sight, but using tags like @master or @latest is not secure. It might lead to unknown results for our end users. Docs: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#using-third-party-actions I don't want to promote this practice. Please, let's consider publishing our GitHub Action with releases. Docs: https://docs.github.com/en/actions/creating-actions/publishing-actions-in-github-marketplace
  2. Our Action is not tested. I test all my custom Actions on each commit, because we can unexpectedly break something: either mypy's CLI or Action spec definition. Example:
  1. I don't understand how install_project_dependencies works. It does not look like it supports poetry / pip-sync / pipenv / custom requirements files / etc. Do we really need this?
  2. I don't know how I can describe profits of using our own GitHub action over these lines:
name: mypy

jobs:
  lint:
    steps:
    - uses: actions/checkout@v2
    - uses: actions/setup-python@v2
      with:
        python-version: 3.9.9
    - run: pip install mypy
    - run: mypy your_path

In the example above you control everything. Python version, extra deps and how to install them, mypy version, its config, security, caching, even custom reporters like https://github.com/reviewdog/
And it is as easy as it gets. While our action does not have all these features. What's our pitch? Why would someone use it over a custom one?

  1. I can imagine that our "pitch" can be: "use master-branch version of mypy with all of the latest features", but it is also not true, because our action uses pip install with ==, so only published versions count.

Do others share my concerns?

@elprans
Copy link
Contributor Author

elprans commented Dec 21, 2021

Please, let's consider publishing our GitHub Action with releases

+1

Our Action is not tested

I can follow up with a test.

I don't understand how install_project_dependencies works.

It uses pip, which should work with any PEP 517-compliant backend.

I don't know how I can describe profits of using our own GitHub action over these lines:

Realistically, you almost always also want to install your dependencies and run mypy with --install-types, which makes the whole thing more like:

name: mypy

jobs:
  lint:
    steps:
    - uses: actions/checkout@v2
    - uses: actions/setup-python@v2
      with:
        python-version: 3.9.9
    - run: pip install -e .
    - run: pip install mypy
    - run: mypy --install-types --non-interactive your_path

It's subjective, of course, but a - uses: python/[email protected] one-liner is a lot easier to add by heart, whereas the above will have to be copy-pasted from docs or other projects, and non-trivial workflow copy-pastes tend to drift away from the recommended best practice.

Lastly, this action is modeled after black's, so there's precedent.

tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
Define a Github Action that can be used as a canonical way of running
mypy as part of Github CI workflows as easy as:

```
    jobs:
      lint:
        runs-on: ubuntu-latest
        steps:
          - uses: python/mypy
```

The action is inspired by its equivalent in black and supports the
following options:

```
    with:
       options: "..."  # mypy command-line options
       paths: "."  # a list of paths to check
       version: "0.910"  # mypy version to use
       install_types: yes|no  # whether to auto-install stubs
```
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
This was broken by python#11320

Co-authored-by: hauntsaninja <>
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.

6 participants