Maintenance: remove outdated Conan 2 linter #23757
Conversation
There was a problem hiding this comment.
please don't, you're removing pylint alltogether, which catches real bugs.
Instead, please remove individual checkers from https://github.com/conan-io/conan-center-index/blob/master/linter/conanv2_transition.py
| if: steps.changed_files.outputs.any_changed == 'true' | ||
| run: | | ||
| echo '## Linter summary (recipes)' >> $GITHUB_STEP_SUMMARY | ||
| pylint --rcfile=linter/pylintrc_recipe `ls recipes/*/*/conanfile.py | shuf -n 500` --output-format=json --output=recipes.json --score=y --exit-zero |
There was a problem hiding this comment.
The linter/pylintrc_recipe is only used here and mentioned in docs/linters.md and docs/developing_recipes_locally.md. So it could be removed as well.
| if: steps.changed_files.outputs.any_changed == 'true' | ||
| run: | | ||
| echo '## Linter summary (test_package)' >> $GITHUB_STEP_SUMMARY | ||
| pylint --rcfile=linter/pylintrc_testpackage `ls recipes/*/*/test_package/conanfile.py | shuf -n 500` --output-format=json --output=recipes.json --exit-zero |
There was a problem hiding this comment.
Same for linter/pylintrc_testpackage. It could be removed as well.
| - name: Run linter | ||
| if: steps.changed-files.outputs.any_changed == 'true' | ||
| run: | | ||
| echo "::add-matcher::linter/recipe_linter.json" |
There was a problem hiding this comment.
The file linter/recipe_linter.json is only used by this file. So it could be removed as well.
|
Once again, pylint is very useful to catch bugs. please consider #23758 which should drop conan1 support for the linter |
Could you provide examples of bugs that have been recently raised by the linter where this has been useful? I'm assuming these fall under the category of "all methods in the recipe actually work, but it may not be working as intended? |
|
any typo in variable/attribute/function name in a non-taken branch can only caught by a static analyzer. not very recent, but still: #7749 (comment) |
|
I meant recent examples where the linter is actively reporting actual recipe bugs in GitHub PRs. Happy to consider the removal on that basis. We are currently working on a revamped pipeline, which will centralise all reporting (linting and builds, and all checks) - so functionality will be assumed and reported by our CI. |
|
Maybe you did not see the edits to my message ? |
PRs from 2 years ago that look like manual runs of the linter on code that was checked in before that? I'm not arguing the linter isn't useful - but I want to measure how often and how recently and exactly what sort of errors it is catching. Otherwise, if it's covering low probability stuff, we can live with it disabled until our new implementation. |
|
Can you please update the description of the PR so that the community understands why this change is needed ? You are not removing outdated conan 2 linters, you are removing ALL PYTHON LINTERS.
In addition to catching bug which are not detected other wise, running pylint has a great advantage: It provides very quick feedback (in the 20 seconds ballpark) on syntax errors, in a very readable way on github (directly in the source code): Also, it does not cost you anything, it does not run on your CI servers, it does not add any traffic on conan-center, and the maintenance is very low (see the number of commits in https://github.com/conan-io/conan-center-index/commits/master/linter and https://github.com/conan-io/conan-center-index/commits/master/.github/workflows/linter-conan-v2.yml) Removing pylint is a net loss in quality, and I see no valid reason to take this move. |
|
I think the benefit of the Python linters to reviewers and maintainers is likely worth the cost. I've definitely benefited from it in my PR's in the past. |
No longer relevant:
Historically, this was useful when there was no Conan 2.0 pipeline running.