[code quality] new make target that combines style and quality targets
Author: stas00
Pull request: yes

**edit**: this post has been edited to reflect the outcome of the discussion.

Any reason why we don't run `flake8` in `make style`? I find myself needing to run `make style` followed by `make quality` all the time, but I need the latter just for the last 2 checks. Since we have no control over the source code why bother with separating checking and fixing - let's just have one target that fixes and then performs the remaining checks, as we know the first two have been done already.

This PR suggests to create a new target `fixup` that combines the 2 separate fix and check functions into one efficient target, 

I will edit the docs if this change resonates with the team.

p.s. if it feels wrong to merge fixing and checking, can we add a 3rd target that is a merged one? `make best`

p.p.s. I know I can make my own alias, I love `make`!

Comments:
I'm in favor. @sgugger @LysandreJik should this be a third target or should we just remove `make quality`?

# [Codecov](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=h1) Report
> Merging [#7310](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=desc) into [master](https://codecov.io/gh/huggingface/transformers/commit/0804d077c634b2149b833ecc7897959cab8bf650?el=desc) will **decrease** coverage by `1.53%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/huggingface/transformers/pull/7310/graphs/tree.svg?width=650&height=150&src=pr&token=9qOlN6Hb1c)](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #7310      +/-   ##
==========================================
- Coverage   78.14%   76.61%   -1.54%     
==========================================
  Files         181      181              
  Lines       35759    35759              
==========================================
- Hits        27945    27396     -549     
- Misses       7814     8363     +549     
```


| [Impacted Files](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/transformers/configuration\_reformer.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3JlZm9ybWVyLnB5) | `21.62% <0.00%> (-78.38%)` | :arrow_down: |
| [src/transformers/modeling\_reformer.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19yZWZvcm1lci5weQ==) | `16.71% <0.00%> (-77.89%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_funnel.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9mdW5uZWwucHk=) | `18.53% <0.00%> (-75.51%)` | :arrow_down: |
| [src/transformers/modeling\_tf\_electra.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ190Zl9lbGVjdHJhLnB5) | `24.25% <0.00%> (-73.56%)` | :arrow_down: |
| [src/transformers/modeling\_layoutlm.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9tb2RlbGluZ19sYXlvdXRsbS5weQ==) | `25.06% <0.00%> (-69.40%)` | :arrow_down: |
| [src/transformers/activations\_tf.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9hY3RpdmF0aW9uc190Zi5weQ==) | `54.16% <0.00%> (-20.84%)` | :arrow_down: |
| [src/transformers/configuration\_layoutlm.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2xheW91dGxtLnB5) | `80.00% <0.00%> (-20.00%)` | :arrow_down: |
| [src/transformers/tokenization\_auto.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy90b2tlbml6YXRpb25fYXV0by5weQ==) | `83.58% <0.00%> (-8.96%)` | :arrow_down: |
| [src/transformers/configuration\_bart.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX2JhcnQucHk=) | `90.00% <0.00%> (-4.00%)` | :arrow_down: |
| [src/transformers/configuration\_utils.py](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree#diff-c3JjL3RyYW5zZm9ybWVycy9jb25maWd1cmF0aW9uX3V0aWxzLnB5) | `96.66% <0.00%> (-0.67%)` | :arrow_down: |
| ... and [16 more](https://codecov.io/gh/huggingface/transformers/pull/7310/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=footer). Last update [0804d07...3c59813](https://codecov.io/gh/huggingface/transformers/pull/7310?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


I like having a command that does the same check as the CI without changing anything so I'd leave `make quality` (basically some docstring formatting is sometimes necessary to have beautiful docs but against what black wants so `make style` can be destructive, it's not jsut styling).

I have no objection with making `make style` do the checks too so that in most cases, we can just do `make style`.

OK, @sgugger - I changed it to `style` and re-used the same intermediary make target so it's one source to change.

BTW, the newly introduced `utils/check_copies.py` takes forever to run :( So perhaps I will still need some custom alias, as it is too slow for a quick check and push cycle.

It appears that the optimal target for quick check-n-push at the moment is:

```
        black examples templates tests src utils
        isort examples templates tests src utils
        flake8 examples templates tests src utils
```

and then rely on CI to do the slow check.


Oh? On my setup it's faster than flake8 so didn't try to optimize. But I can try to make some speedups to that script (basically one regex on the whole content to check whether the for loop is necessary and quickly dimiss files with no copies). It would still open all files if that's where the slowdown comes from though.

It's 2-3 times slower on my machine:
```
$ time python utils/check_copies.py

real    0m26.997s
user    0m24.928s
sys     0m2.052s

$ time flake8 examples templates tests src utils

real    0m11.735s
user    1m47.922s
sys     0m1.051s
```
flake is slow, and the new script is **very** slow

So, I'm not sure how welcome the change to `make style` will be if it's going to be 10 times slower.

Here an alt solution with a new 3rd target `fixup`:
```
quality_checks:
	flake8 examples templates tests src utils
	python utils/check_copies.py
	python utils/check_repo.py

quality:
	black --check examples templates tests src utils
	isort --check-only examples templates tests src utils
	${MAKE} quality_checks

# Format source code automatically and check is there are any problems left that need manual fixing

style:
	black examples templates tests src utils
	isort examples templates tests src utils

fixup: style quality_checks
```

I'm not attached to the name - just looking for something short and intuitive

I don't have a very strong opinion on either adding flake8 to style or having fixup, as long as we keep make quality as a script that does not make any change itself.

And I'm observing the same behaviour with the `utils/check_copies.py`. It takes a while now.

Will speed up the `utils/check_copies.py` today. The lag might be due to the fact we have more copies to check now.

OK, so I went with adding a new target `fixup` that performs automatic fixes and manual checks where automation is not possible. That will not take away the much quicker `make style` from those who don't make coding errors and want just the quick autoformatter.

Added documentation.