Pylint has always been a mix-bag to me. There are a lot of good checks
that help prevent intermediate level errors, but there are also too many
opinionated checks enforcing superfluous metrics (some I’d even say
wrong, e.g. too-few-public-methods and too-many-return-statements).
Airflow’s Python code (more specifically SQLAlchemy’s query DSL) makes
Pylint especially confused since it doesn’t (can’t?) distinguish cases
like `query.filter(DagRun.external_trigger == False` to `if allowed ==
False:`. Yes, you can tell Pylint to ignore things, and explicit is
better than implicit, but Airflow’s code base right now has so many of
those it is at the point that developers are almost writing code to
satisfy Pylint rather for humans to read. The goal of tools is to help
people, and Pylint is hurting more than helping me, personally.
FWIW, most (all? not sure) of the Python projects I’m involved with
don’t use Pylint, and they still maintain good code quality fine. Very
few open source contributors actually make mistakes like `if allowed ==
False` in the first place, and the manual conversation needed to correct
them is very minimal when it occasionally happens. And Pylint don’t
actually prevent that round of review for Airflow, but simply turning it
from “hey this line is not idiomatic Python can you fix that” to “hey
you need to do this to make Pylint happy or we can’t merge”.
So overall I am also +1 to removing Pylint. Or, if we must keep it, I
would strongly encourage disabling checks that we have too many in-line
exceptions for (e.g. the two mentioned above, comparison-with-callable,
comparison-with-singleton, too-many-public-methods, the list goes on).
And yes, that means we need to manually review those `if allowed ==
False` mistakes, but that sacrifice is well worth the benefits IMO.
