added deprecated fields #637
Conversation
| @@ -1242,6 +1243,7 @@ Field Name | Type | Description | |||
| <a name="schemaXml"></a>xml | [XML Object](#xmlObject) | This MAY be used only on properties schemas. It has no effect on root schemas. Adds Additional metadata to describe the XML representation format of this property. | |||
| <a name="schemaExternalDocs"></a>externalDocs | [External Documentation Object](#externalDocumentationObject) | Additional external documentation for this schema. | |||
| <a name="schemaExample"></a>example | Any | A free-form property to include a an example of an instance for this schema. | |||
| <a name="schemaDeprecated"></a> deprecated | `boolean` | Specifies that a schema is deprecated and should be transitioned out of usage. | |||
There was a problem hiding this comment.
As mentioned by @IvanGoncharov, this needs more clarification.
You don't actually deprecate a schema, you deprecate a property in a object model (or you deprecate a parameter, or a whole path/operation, but this is a different issue).
The problem (inherited from JSON Schema) is that an object's property consists of just a name and a schema of its value, and thus we can only add stuff to the value schema, as done here.
Maybe add something similar to the "schemaXml" cited above:
This MAY be used only on properties schemas. It has no effect on root schemas. If used with a value of
true, it specifies that this property is deprecated and should be transitioned out of usage.
An alternative would be to do this similar to required and add a property to the containing schema, with a list of deprecated properties.
In addition, when something is deprecated I often feel the need to mention why it is deprecated, and what to use instead. Of course this can be put into the description, but maybe it is better to make this a string property, with the value being the deprecation documentation? (When using the alternative mentioned above, it could be made an object mapping property name → deprecation info, instead of an array of property names.)
There was a problem hiding this comment.
but maybe it is better to make this a string property, with the value being the deprecation documentation?
@ePaul It was discussed here: #540
And it's a slippery slope since next thing people will ask to implement is the date of deprecation.
IMHO it's very simple rule of thumb if you can programmatically act based on field value; but if it's a text for a user to read then why not use description for that.
One thing that could be added to Swagger spec is a recommendation to add full content of description field into the text of deprecation warning.
There was a problem hiding this comment.
Thanks for the link. You are right, I guess putting it in the description is enough. (And the recommendation to do so should be here.)
There was a problem hiding this comment.
I think such recommendations should be covered outside the spec and within some best practice docs - #589.
|
Deprecating a model that used for POSTing a request body might make sense if an alternate format is introduced. |
So this is deprecating one of the |
|
@darrelmiller - can you elaborate? Wouldn't the method signature change as well? This may very well be related to what defines a unique operation (including potentially content negotiation as @ePaul mentioned). 👍 for parameter deprecation. I understand @ePaul's comment regarding it only being effective in a property definition and not a top-level schema. However, the downside of this is that if there's a model being referenced from multiple places, each referencing place will have to declare it as deprecated. When it comes to reusable component management, this doesn't allow the model to be declared as deprecated so its potential users know not to use it. So 👍 for model-level deprecation as well, but we should consider the implications on various use cases, and perhaps this would introduce additional limitations to it. |
|
@webron Maybe my wording was suboptimal ... in my imagination, top-level schemas become property schemas as soon as they are referenced by some property of another schema. Thus adding the |
|
I don't see harm in deprecating a model. I'd like to propose the intent of this PR stays, it applies to models and model properties. |
|
@OAI/tdc please approve or reject |
|
👍 |
|
My only suggestion is to provide a All that said, I'm a 👍 for 3.0, it seems like a big onion to peel to make it work for every scenario, and the description does the job. |
|
One more thing to add: this also leads us into a documentation-only concern. I worry that this isn't useful in any codegen scenarios. IMO I think we need a bigger discussion about where the line is drawn between contract vs documentation format. |
|
@jasonh-n-austin Codegen can generate deprecated-annotations (if the target language supports them), or could have an option to omit all deprecated elements. |
|
closing as approved |
|
One advantage to using a date rather than a boolean is that once you know of a planned deprecation in the future, you can indicate when it is being retired. I don't have a strong opinion on this issue and will abstain from voting. |
added deprecated fields [Manually ported merge] Co-authored-by: Tony Tam <[email protected]>
Fixes #524