search

python-lsp / python-lsp-server

Fork of the python-language-server project, maintained by the Spyder IDE team and the community
MIT License
1.9k stars 197 forks source link

Remove pylint configuration file #516

Open doolio opened 8 months ago

doolio commented 8 months ago

Project now uses ruff for linting so this config file is obsolete.

ccordoba12 commented 8 months ago

It seems pylintrc is used in a couple of tests. So you need to move it to the tests directory instead and update the respective tests.

doolio commented 8 months ago

But is that not because we still install pylint and pycodestyle? Should these have not been removed as pyflakes and black were?

https://github.com/python-lsp/python-lsp-server/blob/fc2851a23b840481eb92ac522da5bc85305e9d8e/.github/workflows/static.yml#L38-L42

cc: @tkrabel

tkrabel commented 8 months ago

But is that not because we still install pylint and pycodestyle? Should these have not been removed as pyflakes and black were?

https://github.com/python-lsp/python-lsp-server/blob/fc2851a23b840481eb92ac522da5bc85305e9d8e/.github/workflows/static.yml#L38-L42

cc: @tkrabel

I came to the conclusion that pylint is still used in some tests because of the pylint plugin. That’s why it needs to be installed, requiring also pycodestyle. I won’t resist removing that if you find a way to make all tests pass :)

doolio commented 8 months ago

I guess it makes sense we install pylint to test the pylint plugin but then I don't understand then why we no longer install pyflakes to test the pyflakes plugin (and perhaps the flake8 plugin)? Or the other tools (autopep8, mccabe, pydocstyle, rope and yapf for their respective plugins for that matter? I'm still a beginner so forgive my ignorance here.

In any case, I think we can still remove this config file and have a [tool.pylint] section in the pyproject.toml (which pylint supports) at the very least. This way we have only the one config file to maintain.

ccordoba12 commented 8 months ago

@doolio, I think you're misunderstanding things a bit. If you look at one of the failing tests

imagen

you'll see its workspace is set at the root of the repo to get access to its pylintrc file. So, you need to move that file to another place inside our tests directory and update that test accordingly.

doolio commented 8 months ago

I think you're misunderstanding things a bit.

Very likely so I appreciate you taking the time. These tests are failing because I have removed the .pylintrc with this PR and presumably do not fail if it is present, right? So I think pylint searches up the directory tree until it finds its configuration file whether that be a .pylintrc or the pyproject.toml though the former takes precedence. Therefore, I think if I add the pylint configuration to the pyproject.toml the tests should pass If they were passing before, no?

My aim with this PR was to remove .pylintrc so we have less configuration to maintain.

ccordoba12 commented 8 months ago

Therefore, I think if I add the pylint configuration to the pyproject.toml the tests should pass If they were passing before, no?

That should be the case, yes. But I don't think it's necessary because we're not using Pylint for the project anymore. That's why I propose to move the current pylintrc to our test folder and update the failing tests accordingly.