10000 MNT Add pre-comit configuration by rth · Pull Request #16957 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MNT Add pre-comit configuration #16957

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 23, 2020
Merged

MNT Add pre-comit configuration #16957

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
fe90833
Add pre-comit configuration
rth Apr 18, 2020
399e544
Fix doc syntax
rth Apr 18, 2020
08fb033
Update doc/developers/contributing.rst
rth Apr 18, 2020
739adc0
Update .pre-commit-config.yaml
rth Apr 18, 2020
777a11d
Fix line lenght
rth Apr 18, 2020
0caf4b7
Remove --follow-imports skip option in mypy
rth Apr 18, 2020
a457efc
Added mention of git commit -n
rth Apr 19, 2020
File filter 8000

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
repos:
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v2.3.0
hooks:
- id: check-yaml
- id: end-of-file-fixer
- id: trailing-whitespace
- repo: https://gitlab.com/pycqa/flake8
rev: 3.7.8
hooks:
- id: flake8
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this follow the configuration in examples/.flake8 when linting examples?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well right now this only checks for one single error (F401: unused imports) which is also compatible with examples/.flake8. Now it's more a proof of concept for flake8, I'd be happy extend it once we have this pre-commit config file merged :)

types: [file, python]
# only check for unused imports for now, as long as
# the code is not fully PEP8 compatible
args: [--select=F401]
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.730
hooks:
- id: mypy
args:
- --ignore-missing-imports
files: sklearn/
25 changes: 17 additions & 8 deletions doc/developers/contributing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,19 +248,28 @@ modifying code and submitting a PR:
and start making changes. Always use a feature branch. It's good
practice to never work on the ``master`` branch!

9. Develop the feature on your feature branch on your computer, using Git to
do the version control. When you're done editing, add changed files using
``git add`` and then ``git commit``::
9. (**Optional**) Install `pre-commit <https://pre-commit.com/#install>`_ to
run code style checks before each commit::

$ git add modified_files
$ git commit
$ pip install pre-commit
$ pre-commit install
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you might need to mention that occasionally (particularly when creating merge commits), you might need to use git commit -n to disable pre-commits.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What error message will users get the first time?
If it's explicit enough so that a simple google search give them the answer, I'd leave it out.

This section is aimed at new contributors and we need to be careful about not overloading with information (that was an issue in the previous version of the guide that we tried to trim down). Also I honestly doubt they'll ever remember "oh right I saw that in the UG"


to record your changes in Git, then push the changes to your GitHub
account with::
pre-commit checks can be disabled for a particular commit with
`git commit -n`.

10. Develop the feature on your feature branch on your computer, using Git to
do the version control. When you're done editing, add changed files using
``git add`` and then ``git commit``::

$ git add modified_files
$ git commit

to record your changes in Git, then push the changes to your GitHub
account with::

$ git push -u origin my_feature

10. Follow `these
11. Follow `these
<https://help.github.com/articles/creating-a-pull-request-from-a-fork>`_
instructions to create a pull request from your fork. This will send an
email to the committers. You may want to consider sending an email to the
Expand Down
0