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

Add json formatter to pre-commit #571

Merged
merged 5 commits into from
May 12, 2023
Merged

Conversation

emmastephenson
Copy link
Collaborator

@emmastephenson emmastephenson commented May 11, 2023

PULL REQUEST

Summary

Add JSON formatter to our pre-commit hooks.

Related Issue

n/a

Additional Information

Note to reviewers: The only new code is in .pre-commit-config.yaml. Everything else is linting updates!

We have a lot of JSON files floating around (mostly test FHIR bundles, but a couple configs and schemas too) and it would be nice not to need to manually format them.

As far as I can tell, we don't already have a JSON linter on this repository, so I just defaulted to the one pre-commit offers (pretty-format-json).

I also ran this on all the files in the repository to make sure our existing JSON files are formatted. (pre-commit run --all-files, for the curious)

Once this merges, folks may need to run pre-commit autoupdate to get the hook working - at least, I needed to run it for some reason.

@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #571 (a1090e7) into main (6eedc14) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #571   +/-   ##
=======================================
  Coverage   96.49%   96.49%           
=======================================
  Files          45       45           
  Lines        2484     2484           
=======================================
  Hits         2397     2397           
  Misses         87       87           
Flag Coverage Δ
unit-tests 96.49% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

hooks:
- id: pretty-format-json
args: [ --autofix, --no-sort-keys ]
exclude: ".*valid.*"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Review note: we have a couple of intentionally invalid JSON for tests. This makes sure they won't be flagged in future commits, as they conveniently both have valid in the filename (invalid_json and not_valid_json_test, IIRC)

Copy link
Collaborator

@robertmitchellv robertmitchellv left a comment

Choose a reason for hiding this comment

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

i like the prettier format for json, great 💡

Copy link
Contributor

@BradySkylight BradySkylight left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for adding this

@emmastephenson emmastephenson merged commit 299fd2c into main May 12, 2023
@emmastephenson emmastephenson deleted the emma/add-json-formatter branch May 12, 2023 15:30
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.

3 participants