You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This is not a Schema Inaccuracy per se, more of a comment. Let me know if you'd prefer to discuss this somewhere else, or close it as won't fix if you don't see a way to address the problem.
The current schema for #/components/schemas/issue and #/components/schemas/issue-simple defines the body key as optional. I assume the reason is that the respective endpoints support alternative media type formats: text, html, full. Depending on the set media type either body, body_text, body_html, or all three properties are set.
I don't have a solution to this, the definitions are correct given today's constraints. But it results in friction for Octokit users because code such as comment.body.match(regex) will result in a TypeScript compilation errors, because in theory comment.body might be undefined (although it won't in most cases).
I think the most elegant solution would be to set the response Content-Type header to include the requested media type format, e.g. respond with application/vnd.github.v3.html+json instead of application/json, because that would permit us to define different schemas for each supported media type format. For Octokit, I could set the correct response type depending on the requested media type format (if any), so the current friction for integrators would go away. But I guess changing the response Content-Type that would be a breaking change for the API?
Unfortunately I don't know how to work around this problem on Octokit's side. It might be possible, but only with significant initial and ongoing maintenance efforts.
A potential compromise solution would be to define the different response schemas in the OpenAPI spec, even though the API does not set the media type format-specific Content-Type headers (yet). For Octokit, all I would need are the definitions, because this is a compile-time, not a run-time problem.
This discussion was converted from issue #113 on December 08, 2020 21:57.
Heading
Bold
Italic
Quote
Code
Link
Numbered list
Unordered list
Task list
Attach files
Mention
Reference
Menu
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
This is not a Schema Inaccuracy per se, more of a comment. Let me know if you'd prefer to discuss this somewhere else, or close it as won't fix if you don't see a way to address the problem.
The current schema for
#/components/schemas/issue
and#/components/schemas/issue-simple
defines the body key as optional. I assume the reason is that the respective endpoints support alternative media type formats:text
,html
,full
. Depending on the set media type eitherbody
,body_text
,body_html
, or all three properties are set.I don't have a solution to this, the definitions are correct given today's constraints. But it results in friction for Octokit users because code such as
comment.body.match(regex)
will result in a TypeScript compilation errors, because in theorycomment.body
might be undefined (although it won't in most cases).I think the most elegant solution would be to set the response Content-Type header to include the requested media type format, e.g. respond with
application/vnd.github.v3.html+json
instead ofapplication/json
, because that would permit us to define different schemas for each supported media type format. For Octokit, I could set the correct response type depending on the requested media type format (if any), so the current friction for integrators would go away. But I guess changing the response Content-Type that would be a breaking change for the API?Unfortunately I don't know how to work around this problem on Octokit's side. It might be possible, but only with significant initial and ongoing maintenance efforts.
A potential compromise solution would be to define the different response schemas in the OpenAPI spec, even though the API does not set the media type format-specific
Content-Type
headers (yet). For Octokit, all I would need are the definitions, because this is a compile-time, not a run-time problem.Beta Was this translation helpful? Give feedback.
All reactions