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

Fix precommit regex (and format / linter errors) #863

Merged
merged 10 commits into from
Nov 2, 2017

Conversation

DropD
Copy link
Contributor

@DropD DropD commented Oct 27, 2017

In .pre-commit-config.yaml:

  • the file: regex were reformatted to the style documented at http://pre-commit.com/#regular-expressions
  • since every sub-expression in the regex must now match the full path, folders were extended to match path/to/folder/.*.py
  • The format was documented with comments where such a regex first occurs
  • hook versions were updated using pre-commit autoupdate

Other changes:

  • all other changes are formatting and linter fixes that previously weren't caught by pre-comit run --all-files

fix #860

@DropD DropD requested a review from ltalirz October 27, 2017 09:54
@DropD DropD requested review from szoupanos and removed request for ltalirz October 27, 2017 11:25
@ltalirz
Copy link
Member

ltalirz commented Oct 27, 2017

Thanks Rico!
I'm not sure whether regular expressions are really needed here, but if it's the best way of splitting the string that's fine. In some corner cases one might have unintended consequences when one isn't careful (e.g. some of the dots in your example are not escaped). But it's fine with me.

There is one issue, namely the

- repo: git://github.com/pre-commit/pre-commit-hooks
  sha: v1.1.1
  hooks:
  - id: check-yaml

leads to the following error on my conda installation

$ git commit -a
[INFO] Installing environment for git://github.com/pre-commit/pre-commit-hooks.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: Command: ('/Users/leopold/Applications/miniconda3/bin/python', '-m', 'virtualenv', '/Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6', '-p', 'python3.6')
Return code: 100
Expected return code: 0
Output:
    Using base prefix '/Users/leopold/Applications/miniconda3'
    New python executable in /Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6/bin/python3.6
    Also creating executable in /Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6/bin/python
    ERROR: The executable /Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6/bin/python3.6 is not functioning
    ERROR: It thinks sys.prefix is '/' (should be '/Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6')
    ERROR: virtualenv is not compatible with this system or executable
    Running virtualenv with interpreter /Users/leopold/Applications/miniconda3/bin/python3.6

Errors:
    dyld: Library not loaded: @rpath/libpython3.6m.dylib
      Referenced from: /Users/leopold/.cache/pre-commit/repo0z0cpahk/py_env-python3.6/bin/python3.6
      Reason: image not found

I searched for it a bit yesterday and didn't find the solution.
Not sure whether you have an idea for a solution. Otherwise, I'll simply have to comment this block out on my machine.

@ltalirz ltalirz self-requested a review October 31, 2017 17:54
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

In case you want to push it: from my point of view using regex is not ideal, but fine.
Otherwise, feel free to wait for @szoupanos

Copy link
Contributor

@szoupanos szoupanos left a comment

Choose a reason for hiding this comment

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

The check seems to find my modified files now. Thanks!

@szoupanos szoupanos merged commit 4e5852f into aiidateam:develop Nov 2, 2017
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

Successfully merging this pull request may close these issues.

Pre-commit tests are skipped
4 participants