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

Begin prototyping data types of fields and optional #173

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

thadguidry
Copy link
Contributor

@thadguidry thadguidry commented Apr 14, 2024

Fixes #157

  • I'm not 100% sure if we should link using ecmascript, or another standard for the data types. WhatWG has Infra https://infra.spec.whatwg.org/#string as an example, but then I didn't see Array. Still it would be nice to link and point to a standard, but that would be JSON, and I couldn't figure out how to make the ReSpec shortcut links work with RFC8259 and so resorted to the shortcut links using ecmascript (but this all could be changed I'm sure if we knew more or had a friend in ReSpec land)
  • Anyways, take a look at the Core Concepts section Entities where I begin to add them along with some extra info Terms Used and references we were missing about JSON standards in general that we talked about throughout the spec.

- I'm not 100% sure if we should link using ecmascript, or another standard for the data types.  WhatWG has Infra https://infra.spec.whatwg.org/#string as an example, but then I didn't see Array.  Still it would be nice to link and point to a standard, but that would be JSON, and I couldn't figure out how to make the ReSpec shortcut links work with RFC8259 and so resorted to the shortcut links using ecmascript (but this all could be changed I'm sure if we knew more or had a friend in ReSpec land)
Copy link

netlify bot commented Apr 14, 2024

Deploy Preview for reconciliation-api-specs ready!

Name Link
🔨 Latest commit 7ed0869
🔍 Latest deploy log https://app.netlify.com/sites/reconciliation-api-specs/deploys/661de3241be65f00084cf1a3
😎 Deploy Preview https://deploy-preview-173--reconciliation-api-specs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@thadguidry thadguidry requested a review from fsteeg April 14, 2024 08:05
@thadguidry thadguidry marked this pull request as ready for review April 14, 2024 08:05
Copy link
Member

@fsteeg fsteeg left a comment

Choose a reason for hiding this comment

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

Formatting looks good, gives nice overview of data type & optional vs. required.

Made one inline suggestion. Also ReSpec complains about the unused required definition. I think omitting required looks good though, so maybe we can just remove the dfn and make it bold instead? Or link to it once from some place?

draft/index.html Outdated Show resolved Hide resolved
thadguidry and others added 2 commits April 16, 2024 10:08
Co-authored-by: Fabian Steeg <[email protected]>
- Also remove duplicate phrasing paragraph for JSON convention used
@thadguidry
Copy link
Contributor Author

Formatting looks good, gives nice overview of data type & optional vs. required.

Thanks!

Made one inline suggestion. Also ReSpec complains about the unused required definition. I think omitting required looks good though, so maybe we can just remove the dfn and make it bold instead? Or link to it once from some place?

Yeah, agree. So, instead, I now have just put a comment inside doc about that, and then made it only bold/italic to match.
Also wrapped paratheses around them to showcase the syntax used in the document. I think this makes things easier to distinguish.

Finally, does anyone have opinions on the linking for the data types? Notice, if you hover or click on "String" or "Array" etc. that it's currently linked to "ecmascript". Is that OK you think? or should we link to some other standard available in xref ?

@thadguidry
Copy link
Contributor Author

I'll instead link the Data Types against the RFC8259 JSON Specification

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.

Use some better HTML/Markdown format structure to show which query response fields are optional
2 participants