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 pre-commit to auto-format JSON #106

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

brettasmi
Copy link
Contributor

Last Friday, I embarrassingly failed to manually parse a JSON because I struggled with its formatting. The JSON in this repo varies in its formatting and can make some of these queries difficult to parse quickly. To help with this issue, this PR adds a pre-commit hook to auto-format any staged JSON using pre-commit and its prettier plugin

In practice it works as follows:

  • user stages unformatted JSON
  • user commits
  • pre-commit runs and detects unformatted JSON
  • pre-commit calls prettier to format JSON
  • pre-commit unstages JSON
  • the commit fails
  • user stages JSON
  • user commits
  • commit succeeds

Since @MarkDWilliams is essentially the only person who commits to this repository, I thought it would be a good test case for translator to see if it could be adopted more widely (e.g. minihackathon repo) if a single-user repo is successful.

I will make comments on the individual files to explain their function.

Please feel free to reject this PR if you think it will inhibit your workflow. It's just a suggestion that I think could make life easier, but I understand it complicates a widely used repo.

@MarkDWilliams, if you choose to merge this PR, it might be a good idea to do the following on a formatting branch after setting up precommit per the README:

pre-commit run --all-files

Bonus: you get a free dark green block on your git profile :)

Copy link
Contributor Author

@brettasmi brettasmi left a comment

Choose a reason for hiding this comment

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

Explanatory comments

@@ -0,0 +1,6 @@
repos:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pre-commit's configuration:

  • sets the requirement to install prettier
  • restricts prettier's domain to JSON

@@ -0,0 +1,3 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prettier's config setting indentation to 2 spaces. Other options here: https://prettier.io/docs/en/options.html

@@ -1 +1,17 @@
{"message": {"query_graph": {"nodes": {"n0": {"ids": ["MONDO:0014007"], "categories": ["biolink:Disease"]}, "n1": {"categories": ["biolink:Disease"]}}, "edges": {"e01": {"subject": "n0", "object": "n1", "predicates": ["biolink:correlated_with"]}}}}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An example of what the prettier-formatted JSON looks like

@@ -1,7 +1,8 @@
biothings-explorer==1.0.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

alphabetizes and adds pre-commit

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.

2 participants