-
Notifications
You must be signed in to change notification settings - Fork 9.1k
Updated Header Object, removed Items Object #897
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
Conversation
@@ -1606,29 +1540,38 @@ Rate-limit headers: | |||
{ | |||
"X-Rate-Limit-Limit": { | |||
"description": "The number of allowed requests in the current period", | |||
"type": "integer" |
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 guess I missed the change that removed 'type' from Parameter Object and requires the use of schema. This makes an OAS document more verbose as seen in this example - one line becomes three when a parameter uses a primitive, which is by far the most common case (boolean, string, number)
I suggest we reconsider this.
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.
This was a tough call that we went back and forward on just a couple of weeks ago. Eventually, we sided on using the schema object for consistency. Yes it is more verbose, however making a clear delineation between what are parameter/header specific properties and what are schema related properties helps in understanding the behavior of a parameter.
It also has the side benefit of simplifying the documentation and tooling implementation. And just between you and I :-) it makes the idea of introducing alternate schemas in the future much more viable.
Other than removing the 4th Parameter Object different for Header Objects, LGTM. |
- Removed headers being required by default. - Added note about header names being case insensitive.
I did not add a validation restriction saying that names SHOULD or MUST be case insensitive. My take is that tools can choose how to behave as producers and consumers. If you feel differently, please add the wording you'd like to be used. |
@@ -741,7 +740,7 @@ A unique parameter is defined by a combination of a [name](#parameterName) and [ | |||
There are four possible parameter types. | |||
* Path - Used together with [Path Templating](#pathTemplating), where the parameter value is actually part of the operation's URL. This does not include the host or base path of the API. For example, in `/items/{itemId}`, the path parameter is `itemId`. | |||
* Query - Parameters that are appended to the URL. For example, in `/items?id=###`, the query parameter is `id`. | |||
* Header - Custom headers that are expected as part of the request. | |||
* Header - Custom headers that are expected as part of the request. Note that [RFC 7230](https://tools.ietf.org/html/rfc7230#page-22) states header names are case insensitive. |
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.
For consistency, should we wrap the "Note" in parentheses like in other places?
@@ -1591,7 +1525,7 @@ myWebhook: | |||
|
|||
|
|||
#### <a name="headersObject"></a>Headers Object | |||
Lists the headers that can be sent as part of a response. | |||
Lists the headers that can be sent as part of a response. Note that [RFC 7230](https://tools.ietf.org/html/rfc7230#page-22) states header names are case insensitive. |
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.
Same as above.
Other than minor nit about wrapping the "Note that" into a "(Note: ...)", LGTM. |
|
All good from my POV, merge when others have approved. 👍 |
|
Merging! 🎉 |
Updated Header Object, removed Items Object
Updated the Header Object to point to the Parameter Object for an exact structure, but added a few changes. There's no reason not to follow the same new structure as we have for parameters (that was the case before pretty much). Content is not duplicated, but referenced with exceptions.
Modified examples in Header Object and Headers Object.
Completely removed the Items Object as the Header Object was the only one referencing it - not needed anymore.