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

Make auto-generated / vendored files pass pre-commit hooks #335

Open
tangkong opened this issue Apr 28, 2022 · 3 comments
Open

Make auto-generated / vendored files pass pre-commit hooks #335

tangkong opened this issue Apr 28, 2022 · 3 comments

Comments

@tangkong
Copy link
Contributor

tangkong commented Apr 28, 2022

Expected Behavior

We want pre-commit hooks to pass

Current Behavior

There are various files that we don't necessarily need to pass checks, and many of our repos have them

for example, versioneer.py uses raw strings in regex that flake8 doesn't like

for line in f.readlines():
            if line.strip().startswith("git_refnames ="):
                mo = re.search(r'=\s*"(.*)"', line)
                if mo:

docs/conf.py also has import orders that might be necessary but isort objects to.

Possible Solution

  • Omit these files from pre-commit hooks?
  • Fix all the files in the same way?

List of files to skip checks on:

  • versioneer
    • versioneer.py
    • {repo}/version.py
  • docs/source/conf.py
  • cookiecutter
    • {repo}/cookiecutter/.*/conf.yml (hutch-python specific?)

Context

we want style. See the related elog PR

Your Environment

all of them, but pcds-5.3.1

@klauer
Copy link
Contributor

klauer commented Apr 28, 2022

For versioneer, my opinion is:

  1. We should do a versioneer install to update the vendored library first
  2. Then we add it to the pre-commit ignore list
  3. Revisit updating if found to be necessary down the line

For conf.py:

  1. I'd rather not bother and just leave it as excluded from pre-commit, personally
  2. Alternatively we can do # isort: skip where necessary
  3. And then a low-effort reformatting for flake8 using black

@ZLLentz
Copy link
Member

ZLLentz commented Apr 28, 2022

I think ignoring these in pre-commit is best practice

@tangkong
Copy link
Contributor Author

Note: versioneer install replaces versioner.py and {repo}/_version.py, but only appends to __init__.py. That file will have to be manually edited and isort-ed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants