Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run code validation checks with pre-commit #974

Closed
7 tasks done
VakarisZ opened this issue Feb 19, 2021 · 3 comments · Fixed by #1083
Closed
7 tasks done

Run code validation checks with pre-commit #974

VakarisZ opened this issue Feb 19, 2021 · 3 comments · Fixed by #1083

Comments

@VakarisZ
Copy link
Contributor

VakarisZ commented Feb 19, 2021

Is your feature request related to a problem? Please describe.
We should run the same or more code validation tools that our Travis runs using https://pre-commit.com/.

Describe the solution you'd like
Pre-commit tool should run a local validation script and verify that new commit doesn't break UT's, doesn't introduce new flake8 warnings etc.

Describe alternatives you've considered
Using git-hooks

  • Add config for pre-commit. (0d) - @mssalvatore
  • Add pre-commit hook for black. Ensure travis CI uses the same version. (0d) - @mssalvatore
  • Add pre-commit hook for flake8. Ensure travis CI uses the same version. (0d) - @mssalvatore
  • Add pre-commit hook for isort. Ensure travis CI uses the same version. (0d) - @mssalvatore
  • Alter linux deployment script to install and run pre-commit. (0d) - @mssalvatore
  • Alter windows deployment script to install and run pre-commit (0.25d) - @VakarisZ
  • Update dev documentation to include instructions about pre-commit and a warning that CI will reject non-compliant code. (0d) - @mssalvatore
@mssalvatore
Copy link
Collaborator

I've had good luck with these hooks in the past:

repos:
  - repo: https://github.com/asottile/seed-isort-config
    rev: v2.1.1
    hooks:
      - id: seed-isort-config

  - repo: https://github.com/pre-commit/mirrors-isort
    rev: v4.3.21
    hooks:
        - id: isort

  - repo: https://github.com/ambv/black
    rev: 19.10b0
    hooks:
      - id: black

  - repo: https://gitlab.com/pycqa/flake8
    rev: 3.8.1
    hooks:
      - id: flake8

The version numbers are probably out of date.

We probably want to run black against all of the python code before installing pre-commit. Otherwise, it will reformat the files you've changed that are not already properly formatted before each commit and potentially make your commits more confusing.

@mssalvatore
Copy link
Collaborator

We also want to run isort against everything. Once all code has been processed through isort, we can re-enable swim verify in our CI script.

@VakarisZ
Copy link
Contributor Author

For isort opened #990

This was referenced Mar 31, 2021
@mssalvatore mssalvatore mentioned this issue Mar 31, 2021
4 tasks
@mssalvatore mssalvatore mentioned this issue Apr 7, 2021
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants