conan-center-index
conan-center-index copied to clipboard
Maintenance: remove outdated Conan 2 linter
No longer relevant:
- Conan 2 is mandatory in all PRs, so invalid syntax will be immediately shown.
Historically, this was useful when there was no Conan 2.0 pipeline running.
Once again, pylint is very useful to catch bugs. please consider https://github.com/conan-io/conan-center-index/pull/23758 which should drop conan1 support for the linter
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: https://github.com/conan-io/conan-center-index/pull/7749#issuecomment-982468172
https://github.com/conan-io/conan-center-index/pull/11238 https://github.com/conan-io/conan-center-index/pull/11239 https://github.com/conan-io/conan-center-index/pull/11213 https://github.com/conan-io/conan-center-index/pull/22158/commits/983176e1c4d6a40bd130a32e851505fb2cc3af87
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 ?
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.
You say No longer relevant:
which is wrong. I agree that the linters that exist only to help transition to conan 2 can be dropped, and I proposed the fix in https://github.com/conan-io/conan-center-index/pull/23758.
you say Historically, this was useful when there was no Conan 2.0 pipeline running.
which is also wrong, pytlint was considered and implemented before conan 2 migration tools and
Dropping all other pylints checks is a very serious reduction on the quality of conan-center recipe.
I obviously cannot go through all the runs of the checkers, with more than 50 runs per day. One recent actual bug catch was in https://github.com/conan-io/conan-center-index/commit/983176e1c4d6a40bd130a32e851505fb2cc3af87#diff-89f04a00c7ace6261a53[…]25526ec09f965d57fa2bf7R97 which from https://github.com/conan-io/conan-center-index/pull/23437 (but it is not visible in the PR because the author force pushed)
Let me repost examples of bugs which actually where merged in CCI, because pylint was not yet implemented:
- double
configure
method : https://github.com/ericLemanissier/conan-center-index/commit/a56405fdf49ef4139f954840150a413175fff316 - usage of square brackets instead of parentheses : https://github.com/ericLemanissier/conan-center-index/commit/2d02614d1faf1ac6af47a862d3da926340c88a71
- double
validate
method : https://github.com/jgsogo/conan-center-index/commit/f0964c08cb920880c794c2593dfa28ac0a8f6a1e - double
validate
method : https://github.com/jgsogo/conan-center-index/commit/6204d71a05121c56f9d1ef2a8548b034af88921d - double
configure
method: https://github.com/conan-io/conan-center-index/pull/936#discussion_r383519485 -this one was not merged, but only caught after 20 minutes of review) - missing import https://github.com/conan-io/conan-center-index/pull/7696/files#diff-75b9f888a7159f587df53d73476654254af05ca90d777c2525335ec2d2c520d2R2
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):
.
It also works when C3I is in maintenance or out of order.
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.