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

Json Checking #930

Merged
merged 13 commits into from
Jan 17, 2025
Merged

Json Checking #930

merged 13 commits into from
Jan 17, 2025

Conversation

bpkroth
Copy link
Contributor

@bpkroth bpkroth commented Jan 13, 2025

Pull Request

Title

Json Checking


Description

Add pre-commit hooks to check and format json/json5.

  • remove prettier config
  • check json5 files
  • preserve comments when formatting
  • rename mlos_bench config files to *.mlos.jsonc

Type of Change

  • ✨ New feature

Additional Comments

See #882 for comments regarding the difficulty in auto-formatting jsonc/json5 while also preserving comments.
We leave this for future work.


@bpkroth bpkroth added the WIP Work in progress - do not merge yet label Jan 13, 2025
@bpkroth bpkroth changed the title Json Formatting Json Checking Jan 17, 2025
@bpkroth bpkroth marked this pull request as ready for review January 17, 2025 19:54
@bpkroth bpkroth requested a review from a team as a code owner January 17, 2025 19:54
@bpkroth bpkroth enabled auto-merge (squash) January 17, 2025 19:54
@bpkroth bpkroth added ready for review Ready for review tests Add or fix unit tests ci and removed WIP Work in progress - do not merge yet labels Jan 17, 2025
@bpkroth
Copy link
Contributor Author

bpkroth commented Jan 17, 2025

Reduced this to just checking as formatting is harder and I'd rather not have half open PRs for too long.

@bpkroth bpkroth merged commit c66f793 into microsoft:main Jan 17, 2025
16 checks passed
@bpkroth bpkroth deleted the json-formatting branch January 17, 2025 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci ready for review Ready for review tests Add or fix unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants