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

Update Prettier pre-commit hook to new repo #640

Closed
paw-lu opened this issue Oct 28, 2020 · 4 comments · Fixed by #639
Closed

Update Prettier pre-commit hook to new repo #640

paw-lu opened this issue Oct 28, 2020 · 4 comments · Fixed by #639
Labels
bug Something isn't working
Milestone

Comments

@paw-lu
Copy link
Contributor

paw-lu commented Oct 28, 2020

Prettier has moved its pre-commit support to github.com/prettier/pre-commit.

@paw-lu
Copy link
Contributor Author

paw-lu commented Oct 28, 2020

Addressed in #639

@staticdev
Copy link
Contributor

staticdev commented Oct 31, 2020

@paw-lu nice hotfix. I got some broken projects by prettier change.

@cjolowicz cjolowicz added this to the 2020.11.15 milestone Oct 31, 2020
@cjolowicz cjolowicz added the bug Something isn't working label Oct 31, 2020
@cjolowicz
Copy link
Owner

cjolowicz commented Oct 31, 2020

Thank you @paw-lu for fixing this! 🎉

FTR, here's the output from a pre-commit session failing due to this issue:

Python 3.9 / windows-latest
nox > pre-commit run --all-files --show-diff-on-failure
[INFO] Initializing environment for https://github.com/prettier/prettier.
[INFO] Installing environment for https://github.com/prettier/prettier.
[INFO] Once installed this environment will be reused.
[INFO] This may take a few minutes...
An unexpected error has occurred: CalledProcessError: command: ('C:\\Users\\runneradmin\\.cache\\pre-commit\\repocvlm9ipw\\node_env-default\\Scripts\\npm.CMD', 'install')
return code: 1
expected return code: 0
stdout: (none)
stderr:
    npm ERR! code ERESOLVE
    npm ERR! ERESOLVE unable to resolve dependency tree
    npm ERR! 
    npm ERR! While resolving: prettier@2.1.2
    npm ERR! Found: remark-parse@8.0.3
    npm ERR! node_modules/remark-parse
    npm ERR!   remark-parse@"8.0.3" from the root project
    npm ERR! 
    npm ERR! Could not resolve dependency:
    npm ERR! peer remark-parse@"^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0" from remark-math@1.0.6
    npm ERR! node_modules/remark-math
    npm ERR!   remark-math@"1.0.6" from the root project
    npm ERR! 
    npm ERR! Fix the upstream dependency conflict, or retry
    npm ERR! this command with --force, or --legacy-peer-deps
    npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
    npm ERR! 
    npm ERR! See C:\npm\cache\eresolve-report.txt for a full report.
    
    npm ERR! A complete log of this run can be found in:
    npm ERR!     C:\npm\cache\_logs\2020-10-29T04_02_01_107Z-debug.log
    
Check the log at C:\Users\runneradmin\.cache\pre-commit\pre-commit.log
nox > Command pre-commit run --all-files --show-diff-on-failure failed with exit code 1
nox > Session pre-commit failed.

https://github.com/cjolowicz/cookiecutter-hypermodern-python/pull/642/checks?check_run_id=1324588833

What's interesting is that pre-commit only failed with Python 3.9 on Windows. I'm guessing this is because we don't cache pre-commit environments on Windows. It's not great that our CI only caught this because of the different handling on Windows. But then again, setting up pre-commit environments does take a lot of time... (Also, interesting that this wasn't caught even though we recently upgraded prettier, #592 / #596.)

The top-level .pre-commit-config.yaml in this repository is still affected by this issue, actually.

@cjolowicz
Copy link
Owner

Reading the PR and issue on the Prettier repo linked in your original post here helps 😉

  • The original pre-commit hook is still in place. So AFAIU the specific failure we saw was not due to the move itself, but due to issues with broken sub-dependencies that led them to create the new repository. This might explain why only Windows was affected, rather than caching on our side.

  • The new repo should greatly speed up environment creation, so anyway it might be worth investigating if we can drop caching from the CI in this repository (not those of generated projects, I'd say) without too much of an impact. In the new repository, versions like v2.1.2 are actually branches, so they can definitely change with every environment creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants