-
-
Notifications
You must be signed in to change notification settings - Fork 258
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
chore: Normalize formatting of the JSON file #365
Conversation
thanks for the PR, if we do that, we also need to modify the validation script in CI. when it fails, it would give a clear error message "format is wrong, it should be XXXX". |
Sounds good! I'll figure out how to check that in python |
Since there are validations for python, go, and php that just validate the schema, I think this make sense as a separate validation. I'm choosing to write it in JS and run via Node because VSCode uses I'm also supporting a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, ok to do this in JS.
We improve documentation and we merge.
@@ -0,0 +1,21 @@ | |||
const fs = require("fs"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add some documentation at the top of the file to say what it does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All set!
|
||
if (process.argv[2] === "--check") { | ||
if (updated !== original) { | ||
console.error("JSON file format is wrong. Run `node format.js --generate` to update."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
explain the expected format (indentation type, comma convention)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it an error log describing the format
thanks @blaine-arcjet |
While working on some other PRs, I noticed that VSCode was trying to reformat a lot of this file. This is the format that it really wants to apply.
I think the comma on separate lines was supposed to make adding new entries easier but it was lost at some point. If those should remain, let me know and I can try converting the items that don't adhere to that style; however, I figured following a common editor formatting would make for less churn in future PRs.