Opened on 04/21/2016 at 10:50:14 AM
Closed on 07/04/2018 at 09:02:37 AM
#3963 closed change (rejected)
Automatic style and correctness checking for Python code
Reported by: | kvas | Assignee: | |
---|---|---|---|
Priority: | Unknown | Milestone: | |
Module: | Sitescripts | Keywords: | |
Cc: | Blocked By: | ||
Blocking: | Platform: | Unknown / Cross platform | |
Ready: | no | Confidential: | no |
Tester: | Unknown | Verified working: | no |
Review URL(s): |
Description (last modified by kvas)
Background
In the discussion preceding the creation of #3949 we've agreed that whatever the style guide, it's much more likely that it will be consistently followed if we have automatic tools for assessing style guide compliance of a piece of code and for enforcing a certain style.
Now after we've updated the style guide to follow PEP8 and updated the Python code to follow the style guide, we would like to automate the maintenance and improvement of our compliance to the guide.
What to change
Develop a pre-push hook for Mercurial that checks the style guide compliance and correctness of the lines that were changed in the files that contain Python code. Install it on the repositories that contain Python source files that are developed/maintained by us. The hook should not allow pushing the code that adds more errors to the changed lines.
- Use flake8 as the basis of the hook.
- We might want to use flake8-diff for only checking the lines that changed.
- The hook should prevent the push if new inconsistencies/errors are added (since it's possible to disable flake8 for one line or for the whole file with directives in comments, we will always have an override in case we really need to push something and the hook is in the way).
Questions
- Which rules of the style guide are ok to violate? E501 (line too long) is probably one of them.
We've created flake8-abp since and started using Tox for automatic testing and linting. The next step to complete this would be to run these checks before anything gets pushed to the master branch. Most likely we'll want to use GitLab CI once the repositories are migrated there so this ticket seems redundant.