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

Formatting tweaks, JSON fixes and tabs->spaces #363

Merged
merged 5 commits into from
Jun 11, 2021
Merged

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Jun 11, 2021

Just reading through the rendered spec on GitHub and found a couple of indents that are being treated as

quoted text

where I don't think it was intentional.

Please check the rich diff to confirm this (the button is at the top right of the files changed window)

@ml-evs
Copy link
Member Author

ml-evs commented Jun 11, 2021

I think this is happening because we have some mixed tabs/spaces in the specification...

@CasperWA
Copy link
Member

The diff also looks wrong now for code/JSON example sections.

@ml-evs
Copy link
Member Author

ml-evs commented Jun 11, 2021 via email

@ml-evs ml-evs changed the title Two formatting tweaks Formatting tweaks, tabs->spaces Jun 11, 2021
@ml-evs ml-evs force-pushed the ml-evs/formatting branch 2 times, most recently from fdb2c32 to bce3cde Compare June 11, 2021 10:39
@ml-evs
Copy link
Member Author

ml-evs commented Jun 11, 2021

So I ended up going a bit overboard here, and have a script that can strip the JSON blocks out of the spec and validate them as JSON (a separate problem to the whitespace). This turned up lots of extra commas and missing quotes. I'm not sure if it's something that is worth adding to the CI itself as it is slightly manual (e.g. it skips our //comments so there are some false positives), but it has at least turned up some errors.

@ml-evs ml-evs changed the title Formatting tweaks, tabs->spaces Formatting tweaks, JSON fixes and tabs->spaces Jun 11, 2021
@ml-evs ml-evs marked this pull request as ready for review June 11, 2021 11:24
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

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

Well ... this was extremely close to being a clean sheet... Also, it's just a comment/question :)

optimade.rst Outdated Show resolved Hide resolved
CasperWA
CasperWA previously approved these changes Jun 11, 2021
Copy link
Member

@CasperWA CasperWA left a comment

Choose a reason for hiding this comment

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

Bravo monsieur 👏 🥳

@ml-evs ml-evs requested a review from shyamd June 11, 2021 16:12
shyamd
shyamd previously approved these changes Jun 11, 2021
@ml-evs ml-evs dismissed stale reviews from shyamd and CasperWA via c24b97a June 11, 2021 16:21
@shyamd shyamd self-requested a review June 11, 2021 16:33
@ml-evs ml-evs merged commit 240ac01 into develop Jun 11, 2021
@ml-evs ml-evs deleted the ml-evs/formatting branch June 11, 2021 16:33
rartino pushed a commit that referenced this pull request Jul 7, 2021
* Two formatting tweaks

* Tabs to spaces and check in CI

* Remove additional commas and add missing quotes in JSON samples

- Manually fix some indentation

* Turned note back into a quote

* Fix indentation for final example
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.

3 participants