-
Notifications
You must be signed in to change notification settings - Fork 132
Prevent formatting symlink file #1144
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
base: main
Are you sure you want to change the base?
Conversation
|
I will automatically update this comment whenever this PR is modified
|
.pre-commit-config.yaml
Outdated
| - id: check-added-large-files | ||
| # Check for common mistakes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ana-sher! It looks like you may have forgotten to skip the yamlfmt hook when you committed your change, but I think that's because you used VSCode to commit this change, rather than making the commit from the command line, where you would be able to specify SKIP.
Can you revert the changes in this file, other than the single line for excluding docs/LICENSE.txt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @chuckwondo, yes, that's exactly what happened 💯 Reverted and fixed by running from the command line.
|
@ana-sher, thanks again for your first contribution! It looks like your branch is out of date with the |
|
Much better :-) I think I found a solution to the yamlfmt issue, if you want to give it a try. It works for me, but that's because I'm running macOS, so if you want to try the following on your Windows machine, that would be great. If it works, I'd like you to add it to this PR so that it is a complete fix for #1143. Please do the following: First, create a file named """Helper script that automatically skips the yamlfmt pre-commit hook on Windows.
This is not intended to be called directly. It is configured to be called by
pre-commit via the configuration in .pre-commit-config.yml for the yamlfmt hook.
Due to https://github.com/google/yamlfmt/issues/263, comments in yaml files are
not handled properly under Windows, so any Windows user that causes yamlfmt to
format yaml files will cause CI build failure because we build under Linux, and
the resulting format will differ, causing file changes during the pre-commit
step, causing build failure.
Therefore, this script avoids calling yamlfmt (via pre-commit) when running on
Windows.
"""
import platform
import subprocess
import sys
if platform.system() != "Windows":
sys.exit(subprocess.call(["yamlfmt", *sys.argv[1:]]))Then, modify the - repo: https://github.com/google/yamlfmt
rev: v0.20.0
hooks:
- id: yamlfmt
entry: python scripts/yamlfmt.py
types_or: [yaml]
exclude: ".*/vcr_cassettes/.*\\.yaml"Notice that I've added Once you've added the new script and modified the pre-commit config, try running |
…-sher/earthaccess into 1143-pre-commit-hook-win-bug
|
It is a good way to solve it until google/yamlfmt#263 will be fixed! Worked as intended on my Windows machine, thank you @chuckwondo , added script to the PR. |
Prevent fixing symlink file
resolves #1143 LICENSE.txt fixing issue with end-of-file-fixer hook on Windows
📚 Documentation preview 📚: https://earthaccess--1144.org.readthedocs.build/en/1144/