Skip to content

Standardize JSON format #21

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

Merged
merged 1 commit into from
Jan 13, 2020
Merged

Conversation

samamorgan
Copy link
Contributor

I noticed that JSON files in this project had many different formatting conventions. I standardized everything and modified the JSON creation scripts to output new formats according to these conventions.

@samamorgan
Copy link
Contributor Author

Also, when testing the Ruby converter, it exposed a few invalids. I fixed those as well, and generated a new swagger.json after those fixes.

Copy link

@eviltrout eviltrout left a comment

Choose a reason for hiding this comment

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

This looks very nice, thank you!

@eviltrout
Copy link

Although it seems like there is a conflict. Can you rebase and try again?

This has many benefits for human-readability and diff size reduction. The conventions followed here align with most IDE/editor built-in formatters.

No JSON data changes made, only formatting. All changes run through a JSON validator.

###Conventions assumed

- No minified JSON
- Newline on object `{}` or array `[]` begin and end
- 2-space indents
- All members indented
- All members on their own line
- Space after separators `:`
- Double-quotes surrounding strings
- Arrays with no members flattened
- Empty line at EOF

Modified Ruby and Javascript YAML converters to match new conventions.
@samamorgan
Copy link
Contributor Author

I have no idea why there was a conflict. I started from the latest commit.

I rebased and force-pushed to keep the timeline clean. Let me know if you see any other issues!

@oblakeerickson
Copy link
Contributor

Maybe it doesn't matter at all but the reason why the json output was in a single line was to keep the file size small because it doesn't need to be human readable. The doc site originally was pretty slow and I didn't want to make the payload any larger. And I wanted to ensure people were going to edit the swagger.yaml file and not the swagger.json file. The only reason the .json file is included is because we are using githubpages for the hostting, other wise I would just have be part of a build step on not part of the repo at all.

The swagger.json file is going from 302K to 656K with this change which ReDoc uses to generate the site, but that probably doesn't really matter and the slowness probably is till with just parsing the json not in downloading it from the server.

Just curious if there was a reason you are wanting to prettify the json output?

@oblakeerickson oblakeerickson merged commit 3cf529a into discourse:master Jan 13, 2020
oblakeerickson added a commit that referenced this pull request Jan 13, 2020
This reverts commit 3cf529a.

This commit breaks production for some reason. Reverting for now so that
we can get prod back up, and then I'll debug locally.
oblakeerickson added a commit that referenced this pull request Jan 13, 2020
This reverts commit 9f82dd8.

Re-applying this commit, now that I've added some tooling to debug json
conversion locally.
@oblakeerickson
Copy link
Contributor

Thanks for these changes. Since I've always just converted the yml directly to json, I never really looked at the json output, but I think overall it would be good to see the actual output if we ever need to debug it and it might help with being actually swagger compliant, so not having all the json in a single line will be helpful.

I added a command line flag for debugging locally to actually use the converted swagger.json file which you can use with node server.js json.

@samamorgan
Copy link
Contributor Author

@oblakeerickson My primary motivation was for diff checking. It's impossible to see a file diff when a 60,000 line file is flattened to one line. Prettifying the output means the version diff is clearly visible between commits, and tracking down issues with JSON becomes far easier.

Secondary motivation is just style standardization. This project has many JSON files, and they were in varying formats. I unified everything to a single style so the eyes know what to expect when these files need to be read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants