john-bodley opened a new issue #9953: URL: https://github.com/apache/incubator-superset/issues/9953
## [SIP] Proposal for strict pylint enforcement ### Motivation Over the past number of years there have been a number of initiatives to improve the quality of the Python code via a number of linters (`flake8`; deprecated), formatters (`black`, `isort`), and type checkers (`mypy`). The one elephant in the room is [`pylint`](https://en.wikipedia.org/wiki/Pylint), a powerful yet sometimes perceived unusable quality checker. Though `pylint` is enabled as part of [CI](https://github.com/apache/incubator-superset/blob/b8eaa114eddc6a42e2b69b7ab88479c3128a52be/.github/workflows/superset-python.yml#L28) and [`tox`](https://github.com/apache/incubator-superset/blob/52285aeb04ccad1611f88afdec92080ae04bdc0e/tox.ini#L142) many of the checks are ignored either [globally](https://github.com/apache/incubator-superset/blob/980dd2fd419893ec5ed9386ca28fa87115a3753d/.pylintrc#L84) or at the file level via: ``` # pylint: disable=C,R,W ``` From previous experiences the value of many of these linters/checkers only materializes when strict enforcement exists for the entire repo (somewhat akin to heard immunity). Hence rather than merely trying to ensure that new code meets a higher bar, we (as a community) need to roll up our sleeves and take the somewhat painstaking approach of systematically re-enabling checks, removing blanket disablement, etc. somewhat akin to how we were able to strictly enforce `flake8` ([PRs](https://github.com/apache/incubator-superset/pulls?q=is%3Apr+%5Bflake8%5D+Resolving)) and `mypy` ([PRs](https://github.com/apache/incubator-superset/pulls?q=is%3Apr+style%28mypy%29+)). Once the entire code base adheres to the strictly enforced rules it is then relatively simple (in terms of effort) to ensure a PR adheres to the necessary checks. ### Proposed Change I propose that by strictly enforcing `pylint` we will drastically improve the quality, readability, and maintainability of the Python code. I have worked on a number of repos which strictly enforce `pylint` (with very few checkers disabled) and unequivocally say that having `pylint` enabled as help to improve the code quality (remedying a number of bugs), readability, etc. Additionally I have not found `pylint` to be unusable as others have stated and do not find it cumbersome to use (once the necessary ground work has been laid). Note the scale and effort involved is greater than any of the previous checkers and thus will require a systematic approach (hopefully undertaken by a number of individuals in the community). The three steps which need to be undertaken (in order) are: 1. Bumping the version of `pylint` to the latest stable version ([2.5.2](https://pypi.org/project/pylint/2.5.2/) at time of writing). Currently we are using `pylint` [1.9.2](https://pypi.org/project/pylint/1.9.2/) which was released on June 6, 2018. 2. Systematically† remove the globally [disabled](https://github.com/apache/incubator-superset/blob/980dd2fd419893ec5ed9386ca28fa87115a3753d/.pylintrc#L84) checkers‡. 3. Systematically† reenable checking for all files, i.e., removing the `# pylint: disable=C,R,W` comments. † Note _systematically_ implies via a slew of PRs which re-enables a checker (or small subset of checkers) and/or file(s) at a time. ‡ Note based on other repos which use `pylint` all checkers could be disabled except for: - `bad-continuation` which is incompatible with `black`. - `missing-module-docstring` since Python modules are typically not documented at the module/file level. ### New or Changed Public Interfaces N/A ### New dependencies N/A ### Migration Plan and Compatibility N/A ### Rejected Alternatives N/A ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
