palantir / python-language-server

An implementation of the Language Server Protocol for Python
MIT License
2.6k stars 282 forks source link

PR: Add mypy plugin to core plugins #838

Open steff456 opened 3 years ago

steff456 commented 3 years ago

This PR migrates the pyls-mypy plugin, https://github.com/tomv564/pyls-mypy, to the core plugins of PyLS for us to maintain

ccordoba12 commented 3 years ago

@uschi2000, I'm also having troubles in this repo to merge pull requests. As you can see, the Appveyor integration is required to pass, but its status is not registered by Github. That's very strange because tests are passing just fine on the Appyevor servers:

https://ci.appveyor.com/project/gatesn/python-language-server/builds/34455591

A possible solution for this would be to move to move testing to Github actions, as proposed by @goanpeca on PR #803.

uschi2000 commented 3 years ago

So it seems that the appveyor integration is done in terms of @gatesn 's user name. Since he is no longer in the Palantir Org this integration probably broke.

uschi2000 commented 3 years ago

@gatesn could you give me a hand in restoring/migrating/fixing the appveyor CI integration for this repo (and python-grpc)? Thanks very much!

mpanarin commented 3 years ago

@steff456 What is the issue with using the plugin? It does its job just fine. I think it is worth to try finishing the https://github.com/palantir/python-language-server/issues/391 for mypy daemon integration, thus making the plugin and mypy api integration obsolete?

Mypy daemon works best for project setups, while the plugin only runs the checker on a single file, missing a lot of type checks coming from imports, etc. as mentioned in the https://github.com/tomv564/pyls-mypy/issues/24

gatesn commented 3 years ago

@uschi2000 not sure what I need to do here? Can you remove appveyor from required checks? I had a look and it says AppVeyor permissions are inherited from GitHub perms. So if you're an admin here you should get admin there.

uschi2000 commented 3 years ago

@gatesn I personally have no idea what the appveyor integration does here and whether we need it or not ;)

ccordoba12 commented 3 years ago

I personally have no idea what the appveyor integration does here and whether we need it or not ;)

@uschi2000, thanks again for your help sorting out these issues.