Grammar and phrasing fixes #82
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -949,7 +949,7 @@ Field Name | Type | Description | |||
| <a name="modelRequired"/>required | [`string`] | A definition of which properties MUST exist when a model instance is produced. The values MUST be the [`{Property Name}`](#propertiesPropertyName) of one of the [`properties`](#528-properties-object). | |||
| <a name="modelProperties"/>properties | [Properties Object](#528-properties-object) | **Required.** A list of properties (fields) that are part of the model | |||
| <a name="modelSubTypes"/>subTypes | [`string`] | List of the [model `id`s](#modelId) that inherit from this model. Sub models inherit all the properties of the parent model. Since inheritance is transitive, if the parent of a model inherits from another model, its sub-model will include all properties. As such, if you have `Foo->Bar->Baz`, then Baz will inherit the properties of Bar and Foo. There MUST NOT be a cyclic definition of inheritance. For example, if `Foo -> ... -> Bar`, having `Bar -> ... -> Foo` is not allowed. There also MUST NOT be a case of multiple inheritance. For example, `Foo -> Baz <- Bar` is not allowed. A sub-model definition MUST NOT override the [`properties`](#modelProperties) of any of its ancestors. All sub-models MUST be defined in the same [API Declaration](#52-api-declaration). | |||
| <a name="modelDiscriminator"/>discriminator | `string` | MUST be included only if [`subTypes`](#modelSubTypes) is included. This field allows for polymorphism within the described inherited models. This field MAY be included at any base model but MUST NOT be included in a sub-model. The value of this field MUST be a name of one of the [`properties`](#modelProperties) in this model, and that field MUST be in the [`required`](#modelRequired) list. When used, the value of the *discriminator property* MUST be the name of parent or any of its sub-models (to any depth of inheritance). | |||
| <a name="modelDiscriminator"/>discriminator | `string` | MUST be included only if [`subTypes`](#modelSubTypes) is included. This field allows for polymorphism within the described inherited models. This field MAY be included at any base model but MUST NOT be included in a sub-model. The value of this field MUST be a name of one of the [`properties`](#modelProperties) in this model, and that field MUST be in the [`required`](#modelRequired) list. When used, the value of the *discriminator property* MUST NOT be the name of the parent model or any of its sub-models (to any depth of inheritance). | |||
There was a problem hiding this comment.
Please verify that the change to the last sentence is the actual intent of the spec, since it changes a MUST to a MUST NOT (effectively changing the spec).
There was a problem hiding this comment.
The original statement is correct. That's what the discriminator is used for.
Say you have these models (simplified sample):
Pet (discriminator:type) { type, name, age }
Cat extends Pet { hasClaws }
Dog extends Pet { likesCats }
In that case, the 'type' field has to be either Pet, Cat or Dog (the model IDs) to indicate which model it is. It can't be any value as that won't be able to be parsed by the consumer/producer.
|
Thank you for this work! I'm really glad you took the time to go over it and make these modifications. |
|
Ah, that makes sense, thanks for clarifying, fixed and pushed the amend. |
fehguy
added a commit
that referenced
this pull request
Sep 8, 2014
fehguy
added a commit
that referenced
this pull request
Sep 8, 2014
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes various grammar and phrasing issues in the spec. Also fixes a seeming typo in the restriction of discriminator property value of models to be the names of the parent model or any of its sub-models. The intent seems to restrict the discriminator property value of models to NOT be the name of the parent model or any of its sub-models. (Let me know if that wasn't the intent and I can revert that change).