-
-
Notifications
You must be signed in to change notification settings - Fork 205
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
Automatically format the JSON as part of CI #268
Comments
Another way would be to introduce something very simple like a |
If we format the JSON automatically, there is one drawback. For example, when someone wants to split the changes into 2 commits, all changes get included in one commit, as we are formatting JSON, and running 'git add filename' (or if I have a knowledge gap at this point, please let me know) . This causes all changes to be included in one commit. So, I think a better option is the same as @jviotti said: we should show an error before 'commit' or 'stage' or on workflow if the file is not formatted properly and provide one command which format all the JSON file well I am interested to work on this |
I'm not a fan of requiring the committer to do extra work (primarily me being the committer). I much prefer that if anything is done here, it's done as an automatic process where the committer is completely uninvolved. Secondarily, a git hook only works locally. Any solution that requires a git hook has to be added on everyone's machine. That leaves one solution: a CI step that runs after merging to We did this on the JSON Path test suite. It's pretty simple. |
My concern about automatic formatting is that most of the time it makes code less readable because it's too generic and can't take context and aesthetics into account. The developer can make better aesthetic choices based. For example, a simple reference is best written on one line: "foo": { "$ref": "#/$defs/bar" } but an auto formatter will generally split it up: "foo": {
"$ref": "#/$defs/bar"
} The I am strongly against automation that automatically overrides the aesthetic choices of a developer. I'd like to see the formatter limited only to rules that are universal such as spacing and indentation and not affect a developer's aesthetic choices such as inserting/removing newlines. Generally I prefer that these things warn the developer what they did that violates the code style and gives them a chance to fix it. Developers will learn the preferred code style better if they make the changes themselves rather than have those changes done for them. If they have to do it themselves, they'll soon stop making the same mistakes. Not having it automatic also allows users to ignore warnings that deliberately violate some rule for aesthetic purposes. However, if everyone prefers auto formatting, I just ask that it be configured to not mess with newlines. ESLint has a lot of rules that can fairly intelligently allow for developer freedom while keeping reasonable boundaries. They can even auto format based on those rules if that's what we want. However ESLint is designed for JavaScript, not JSON. I know there are ways to use ESLint with JSON, but I don't know if you have access to the same richness of rule expression as you do with JavaScript. |
This looks promising, https://ota-meshi.github.io/eslint-plugin-jsonc/. It looks like it covers the basics and has at least some of the more advanced rules that I mentioned implemented, such as this one, https://eslint.org/docs/latest/rules/object-curly-newline. |
See maybe:
https://github.com/typicode/husky
or
https://pre-commit.com/
The text was updated successfully, but these errors were encountered: