[GitHub] [airflow] potiuk commented on issue #5238: [AIRFLOW-4364] Add Pylint to CI

[GitBox <gi...@apache.org>]

potiuk commented on issue #5238: [AIRFLOW-4364] Add Pylint to CI
URL: https://github.com/apache/airflow/pull/5238#issuecomment-489447343
 
 
   Hey @BasPH - I will take a closer look later today as I am very interested :) (travelling today) but I have one question/idea which we might implement with this change. We can defer it for the future as well, but I think maybe it's worth starting now.
   
   Since we are starting to use more linters, I thought maybe we could already switch to using pre-commit-hook framework for those linters.
   
   https://pre-commit.com/
   
   It allows to run checks on CI but (what is more important) it can run the very same checks as pre-commit-hooks. It is really nicely implemented - has nice UI, allows to add many ready-to-use linters and checkers (and some automated code modification like adding licence headers) and it is super-easy to install locally by the developer. And it has pluginable interface where it can already (I believe) filters only changed files (not lines by default though). 
   
   As local pre-commit check, It could be run as pre-commit for all locally modified files, so that people are encouraged to fix error faster. And on Travis we could continue checking only modified lines for example. 
   
   I think you could fairly easily turn your python script into a pre-commit plugin rather than have a standalone script and then we could benefit from being able to run the checks with pre-commit hooks (which is far better than waiting for Travis).
   
   I discovered it recently and applied successfully to the Ooozie2Airflow converter we work on - we applied some 20+ checks. You can see for example here: 
   https://travis-ci.org/GoogleCloudPlatform/cloud-composer/builds/528367055#L1638
   
   And here is the list of checks we have implemented in our project:
   
   ```
   Formats python files using black...................................................Passed
   Add licence for all XML, md files..................................................Passed
   Add licence for all .pig files.....................................................Passed
   Add licence for all python/yaml/property files.....................................Passed
   Add licence for all Jinja templates................................................Passed
   No-tabs checker....................................................................Passed
   Flake8.............................................................................Passed
   Check that executables have shebangs...............................................Passed
   Check for merge conflicts..........................................................Passed
   Check Xml..........................................................................Passed
   Check Yaml.........................................................................Passed
   Debug Statements (Python)..........................................................Passed
   Detect Private Key.................................................................Passed
   Fix python encoding pragma.........................................................Passed
   Fix End of Files...................................................................Passed
   Mixed line ending..................................................................Passed
   Fix requirements.txt...............................................................Passed
   Trim Trailing Whitespace...........................................................Passed
   Check hooks apply to the repository................................................Passed
   Check for useless excludes.........................................................Passed
   Checks typing annotations consistency with mypy....................................Passed
   Checks for common programming errors with pylint...................................Passed
   Runs all unit tests with pytest....................................................Passed
   Check Shell scripts syntax corectness..............................................Passed
   Detect unicode non-breaking space character U+00A0 aka M-BM-.......................Passed
   Remove unicode non-breaking space character U+00A0 aka M-BM-.......................Passed
   Detect the EXTREMELY confusing unicode character U+2013............................Passed
   Remove the EXTREMELY confusing unicode character U+2013............................Passed
   Validates all oozie workflows......................................................Passed
   Checks for security vulnerabilities in dependencies................................Passed
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services