search

andreoliwa / nitpick

Enforce the same settings on multiple projects
https://nitpick.readthedocs.io/
MIT License
393 stars 25 forks source link

.pre-commit-config.yaml file created by nitpick doesn't have the rev key #472

Open khuyentran1401 opened 2 years ago

khuyentran1401 commented 2 years ago

Expected behavior

After running, nitpick fix, a .pre-commit-config.yaml file was created. I expect to see the required key rev under all repos.

Current behavior

No key rev is shown in the .pre-commit-config.yaml file. Here is how the file looks like:

repos:
  - repo: https://github.com/PyCQA/bandit
    hooks:
      - id: bandit
        args:
          - --ini
          - setup.cfg
        exclude: tests/
  - repo: https://github.com/psf/black
    hooks:
      - id: black
        args:
          - --safe
          - --quiet
  - repo: https://github.com/asottile/blacken-docs
    hooks:
      - id: blacken-docs
        additional_dependencies:
          - black==21.5b2
  - repo: https://github.com/PyCQA/flake8
    hooks:
      - id: flake8
        additional_dependencies:
          - flake8-blind-except
          - flake8-bugbear
          - flake8-comprehensions
          - flake8-debugger
          - flake8-docstrings
          - flake8-isort
          - flake8-polyfill
          - flake8-pytest
          - flake8-quotes
          - flake8-typing-imports
          - yesqa
  - repo: https://github.com/pre-commit/pygrep-hooks
    hooks:
      - id: python-check-blanket-noqa
      - id: python-check-mock-methods
      - id: python-no-eval
      - id: python-no-log-warn
      - id: rst-backticks
  - repo: https://github.com/pre-commit/pre-commit-hooks
    hooks:
      - id: debug-statements
  - repo: https://github.com/asottile/pyupgrade
    hooks:
      - id: pyupgrade
        args:
          - --py37-plus
  - repo: https://github.com/PyCQA/isort
    hooks:
      - id: isort
  - repo: https://github.com/pre-commit/mirrors-mypy
    hooks:
      - id: mypy
        args:
          - --show-error-codes
  - repo: local
    hooks:
      - id: pylint
        name: pylint
        language: system
        exclude: tests/
        types:
          - python
  - repo: https://github.com/myint/autoflake
    hooks:
      - id: autoflake
        args:
          - --in-place
          - --remove-all-unused-imports
          - --remove-unused-variables
          - --remove-duplicate-keys
          - --ignore-init-module-imports
  - repo: https://github.com/commitizen-tools/commitizen
    hooks:
      - id: commitizen
        stages:
          - commit-msg
  - repo: https://github.com/pre-commit/pre-commit-hooks
    hooks:
      - id: end-of-file-fixer
      - id: trailing-whitespace
  - repo: https://github.com/pre-commit/mirrors-prettier
    hooks:
      - id: prettier
        stages:
          - commit
  - repo: https://github.com/openstack/bashate
    hooks:
      - id: bashate
        args:
          - -i
          - E006

Steps to reproduce

Run nitpick fix created a .pre-commit-config.yaml file

Context

I can't commit because of the missing required key rev. Here is the messing I saw when committed:

An error has occurred: InvalidConfigError: 
==> File .pre-commit-config.yaml
==> At Config()
==> At key: repos
==> At Repository(repo='https://github.com/PyCQA/bandit')
=====> Missing required key: rev
Check the log at /Users/khuyen/.cache/pre-commit/pre-commit.log

Your environment

$ cat $(which flake8)

#!/Library/Frameworks/Python.framework/Versions/3.8/bin/python3.8
# -*- coding: utf-8 -*-
import re
import sys
from flake8.main.cli import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())
andreoliwa commented 2 years ago

Hey, thanks for the bug report!

TL;DR: the absence of the rev key is actually by design: I don't enforce versions on the built-in styles with pre-commit hooks, because I use https://pre-commit.ci/ for auto-updates on my projects.

Read further below for more details.

Possible solution

This problem should be fixed when #218 will be developed.

  1. Mark the rev key as "should be present" for every pre-commit hook in every built-in style.
  2. Add a default value for each hook as well. It can be the latest tag, or the default branch (master, main, develop... depends on each project).
  3. The developer will be responsible to run pre-commit autoupdate after applying changes with nitpick fix. The built-in styles will only enforce the presence of rev and will not guarantee that the latest tag is being used.

Why the rev key is not set on styles?

I removed all rev keys from the built-in styles because I didn't want to keep updating the styles manually every time a hook got upgraded by https://pre-commit.ci/.

I had to change docs, styles and tests whenever a hook was upgraded. Examples:

It was annoying and there were no auto-updates: I always had to intervene manually.

khuyentran1401 commented 2 years ago

Hi @andreoliwa, thank you for proposing the solutions and for the explanation. I think using numbers 2 and 3 will solve this problem. I didn't know about pre-commit autoupdate until now so it would be nice if you can include that in the documentation

andreoliwa commented 2 years ago

I think using numbers 2 and 3 will solve this problem.

Awesome that these items unblock you. 🥳 But I will still keep this bug open until I am (or someone is) able to fix it for good.

I didn't know about pre-commit autoupdate until now so it would be nice if you can include that in the documentation

Good idea, I'll do that. 👍🏻

AntarktisZ11 commented 2 years ago

First of all, great project! Just want to say that I used the following layout

[[".pre-commit-config.yaml".repos]]
repo = "https://github.com/pre-commit/pre-commit-hooks"
rev = "" # run `pre-commit autoupdate`

Which worked well as a placeholder. Not sure if there are any downsides with this though! If the built-in styles contain that they would work after running pre-commit autoupdate.

valberg commented 1 year ago

Hi y'all!

I'm also running into this issue, and the proposed solution with pre-commit autoupdate does not work in my case:

# nitpick style
[nitpick.files.present]
".pre-commit-config.yaml" = "Create the file with the contents below, then run 'pre-commit install'"

[[".pre-commit-config.yaml".repos]]
repo = "https://github.com/astral-sh/ruff-pre-commit"

[[".pre-commit-config.yaml".repos.hooks]]
id = "ruff"

Trying to run it on a repo without the .pre-commit-config.yaml file

$ nitpick fix
.pre-commit-config.yaml:1: NIP103  should exist: Create the file with the contents below, then run 'pre-commit install'
.pre-commit-config.yaml:1: NIP361  was not found. Create it with this content:
repos:
  - repo: https://github.com/astral-sh/ruff-pre-commit
    hooks:
      - id: ruff
Violations: ✅ 1 fixed, ❌ 1 to change manually.

$ nitpick fix
No violations found. ✨ 🍰 ✨

$ pre-commit autoupdate
An error has occurred: InvalidConfigError: 
==> File .pre-commit-config.yaml
==> At Config()
==> At key: repos
==> At Repository(repo='https://github.com/astral-sh/ruff-pre-commit')
=====> Missing required key: rev
Check the log at /home/valberg/.cache/pre-commit/pre-commit.log

pre-commit expects there to be a rev key. Or maybe I'm missing something?

andreoliwa commented 1 year ago

I'm also running into this issue, and the proposed solution with pre-commit autoupdate does not work in my case:

Maybe it works if you add rev = "master" on your style.

pre-commit expects there to be a rev key. Or maybe I'm missing something?

You're correct. The problem is that I can't add rev = "master" or rev = "" to the built-in styles. Otherwise, they will be enforced by Nitpick.

This still depends on #218 being developed first as I mentioned in https://github.com/andreoliwa/nitpick/issues/472#issuecomment-1079692929. If you see any other solution, let me know.

Not sure if there are any downsides with this though! If the built-in styles contain that they would work after running pre-commit autoupdate.

@AntarktisZ11 Currently, if the style has rev = "" then Nitpick will enforce it and erase existing revisions. 😕

andreoliwa commented 1 year ago

so it would be nice if you can include that in the documentation

I updated the docs: https://nitpick.readthedocs.io/en/latest/troubleshooting.html#missing-rev-key-when-using-the-default-pre-commit-styles