Skip to content

pre-commit: validate json files on commit #12925

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

Merged
merged 2 commits into from
Jul 30, 2025
Merged

Conversation

simonrw
Copy link
Contributor

@simonrw simonrw commented Jul 29, 2025

Motivation

Sometimes I have to update snapshot files manually. For example when moving tests from one file to another. I regularly mess up the syntax so the tests fail once CI has run.

We have a series of pre-commit hooks that are run before code is committed. As part of the "push left" initiative (of which LocalStack itself is part!) we should be able to catch these checks before committing code, and definitely before pushing code up to GitHub.

Changes

  • Add the check-json hook in our pre-commit configuration file

@simonrw simonrw added the semver: patch Non-breaking changes which can be included in patch releases label Jul 29, 2025
@simonrw simonrw added this to the Playground milestone Jul 29, 2025
@simonrw simonrw marked this pull request as ready for review July 29, 2025 21:05
Copy link

Test Results - Preflight, Unit

21 991 tests  ±0   20 257 ✅ ±0   6m 18s ⏱️ -2s
     1 suites ±0    1 734 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 84357fe. ± Comparison against base commit 5535768.

Copy link

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 9s ⏱️ ±0s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 84357fe. ± Comparison against base commit 5535768.

@coveralls
Copy link

coveralls commented Jul 29, 2025

Coverage Status

coverage: 83.157% (+31.8%) from 51.37%
when pulling 84357fe on pre-commit/check-json-files
into 5535768 on main.

Copy link

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 20m 20s ⏱️
4 841 tests 4 361 ✅ 480 💤 0 ❌
4 847 runs  4 361 ✅ 486 💤 0 ❌

Results for commit 84357fe.

Copy link

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   1h 42m 6s ⏱️ -41s
4 482 tests ±0  4 155 ✅ ±0  327 💤 ±0  0 ❌ ±0 
4 484 runs  ±0  4 155 ✅ ±0  329 💤 ±0  0 ❌ ±0 

Results for commit 84357fe. ± Comparison against base commit 5535768.

Copy link
Member

@alexrashed alexrashed left a comment

Choose a reason for hiding this comment

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

I think this is a nice convenience feature which should not have any impact on performance (since pre-commit only checks files in the commit). 💯

@simonrw simonrw merged commit d76a019 into main Jul 30, 2025
40 checks passed
@simonrw simonrw deleted the pre-commit/check-json-files branch July 30, 2025 08:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver: patch Non-breaking changes which can be included in patch releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants