8000 pre-commit: Try to autofix yaml BEFORE checking it by cclauss · Pull Request #269 · pyscript/pyscript · GitHub
[go: up one dir, main page]
Skip to content

pre-commit: Try to autofix yaml BEFORE checking it #269

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 7, 2022
Merged

pre-commit: Try to autofix yaml BEFORE checking it #269

merged 2 commits into from
May 7, 2022

Conversation

cclauss
Copy link
Contributor
@cclauss cclauss commented May 6, 2022 •
edited
Loading

Also fixes an issue where check-yaml was being run twice.

@mattkram
Copy link
Contributor
mattkram commented May 7, 2022

Good catch on check-yaml.

Regarding the ordering of reformat and check yaml, I'm not convinced this would actually change anything. Can you share an example?

My skepticism is that, even if the yaml files are reformatted before the check-yaml hook is run, they are not restaged and thus pre-commit will fail anyway.

If that is the case, then personally I prefer to keep the "official" hooks at the top of the file for aesthetic reasons.

@cclauss
Copy link
Contributor Author
cclauss commented May 7, 2022

I spotted the problem here https://results.pre-commit.ci/run/github/461710233/1651873302.QTUN1cNZROSpFTt5YjKXiA

@mattkram
Copy link
Contributor
mattkram commented May 7, 2022

I spotted the problem here https://results.pre-commit.ci/run/github/461710233/1651873302.QTUN1cNZROSpFTt5YjKXiA

Looks like that error in particular was caused by the use of the symbol "*" without quotes, and unrelated to ordering.

Let's just remove the duplicate check-yaml and move the check-json up to be with its siblings for now.

mattkram
mattkram approved these changes May 7, 2022
View reviewed changes
Copy link
Contributor
@mattkram mattkram left a comment

Choose a reason for hiding this comment

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

🚀 Thanks @cclauss!

@mattkram mattkram merged commit 22ffbcb into pyscript:main May 7, 2022
@cclauss cclauss deleted the patch-1 branch May 7, 2022 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattkram mattkram mattkram approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cclauss @mattkram
0