DOC add command to run pre-commit checks to pull request template

[MarcoGorelli]

I think we could add

passes `pre-commit run --from-ref=upstream/master --to-ref=HEAD --all-files`

to the pull request template (maybe it could even replace the current black and flake8 ones).

The file to change would be

.github/PULL_REQUEST_TEMPLATE.md

Some of the contributing guide may also need updating if it mentions the flake8 --diff command.

[BobinMathew]

Hi @MarcoGorelli
Do you want this on the file?

• closes #xxxx
• tests added / passed
• pre-commit run --from-ref=upstream/master --to-ref=HEAD
• whatsnew entry

[MarcoGorelli]

@BobinMathew yes, but will need "passes" in there too, and --all-files (have updated the issue text)

[BobinMathew]

I gave a PR.
Checking's still continuing.

Update: There seems to be error in
continuous-integration/travis-ci/pr — The Travis CI build could not complete due to an error

[MarcoGorelli]

Great! Can you put the issue number (#38694) in the pull request body (where it says closes #xxxxx) so it gets linked to this issue?

Travis CI error's likely unrelated

[BobinMathew]

Ok @MarcoGorelli , I added the issue number

[MarcoGorelli]

You need to put it in the pull request body - look at other pull requests for an example, e.g. #38668

[BobinMathew]

Hi @MarcoGorelli , I saw some errors.
Is every this alright?

[jorisvandenbossche]

Should we actually recommend that? It's a quite complex line of cli code, and if you are using pre-commit, I suppose you will typically use it as a git commit hook?

[MarcoGorelli]

Is it more complicated than the current flake8 line

git diff upstream/master -u -- "*.py" | flake8 --diff

?

The command I posted gives you a cross-platform way of running all the CI code quality checks, with all the right package versions, only on the files which you've changed. You can run it even if you don't use pre-commit as a git commit hook

[jorisvandenbossche]

Is it more complicated than the current flake8 line

No, but if someone proposed to add that line to the template (if it wasn't there yet), I would probably raise the same concern ;-)
And I personally also never ever used that flake8 line in practice ..

I am basically wondering how many of our core / regular contributors are using that. And if almost no one does (if! I don't know that, to be clear), not sure it should be the first thing to have contributors see when opening a PR.

The command I posted gives you a cross-platform way of running all the CI code quality checks, with all the right package versions, only on the files which you've changed. You can run it even if you don't use pre-commit as a git commit hook

Yes, and that sounds valuable in certain cases.
But then we should probably also add that to and explain it in the contributing docs?

Maybe a general "Ensure all linting tests pass, see here how to run them" with a link to a clear overview in the contributing guide on (different ways) how you can run the linting might be more useful? It's more clicking, but it migth also be less scary as such a complex command without much context.

[MarcoGorelli]

OK, thanks for explaning 😄

Maybe a general "Ensure all linting tests pass, see here how to run them" with a link to a clear overview in the contributing guide on (different ways) how you can run the linting might be more useful?

Sure, sounds good - I think there could be a link to https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards , and there this command could be put in place of ./ci/code_checks.sh (which won't work on Windows and doesn't include all checks), with a short explanation of what it does (i.e. it runs all the code quality checks on all files which have changed between the current commit and upstream/master)

[jorisvandenbossche]

Yep, that sounds good to me