Skip to content

Master#698

Closed
BrianBlum wants to merge 6 commits intoAzure:masterfrom
BrianBlum:master
Closed

Master#698
BrianBlum wants to merge 6 commits intoAzure:masterfrom
BrianBlum:master

Conversation

@BrianBlum
Copy link
Copy Markdown

@BrianBlum BrianBlum commented Nov 8, 2016

Documentation update and proper required parameter settings.


This change is Reviewable

Brian Blum added 4 commits November 3, 2016 09:04
Change summary tags to description tags.
Add proper text to reponse code descriptions
Add examples for responses.
Apply better grammar.
Make the description clearer and mark the id as required
linter caught missing description
@azurecla
Copy link
Copy Markdown

azurecla commented Nov 8, 2016

Hi @BrianBlum, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!


It looks like you're working at Microsoft (brianbl). If you're full-time, we DON'T require a contribution license agreement.



If you are a vendor, DO please sign the electronic contribution license agreement. It will take 2 minutes and there's no faxing! https://cla.azure.com.

TTYL, AZPRBOT;

{
"nameAvailable": true,
"reason": null,
"message": null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Representing null values in the response provides wrong information. Swagger spec says that the properties are not required. That means they may or may not be sent in the response. Sending a null value means a different thing. So please remove them from here.

If the service is actually sending this on the wire then the service should fix it. I would recommend using fiddler to provide examples.

"schema": {
"$ref": "#/definitions/CheckNameAvailabilityOutput"
},
"examples": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is an incorrect format. The example in the responses section should be keyed on the MIME type.
Take a look at what the specification says https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#exampleObject

"description": "Name of the resource group within the Azure subscription.",
"in": "path",
"required": true,
"type": "string"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"x-ms-parameter-location": "method"

"type": "string",
"pattern": "^[a-z0-9]",
"minLength": 3,
"maxLength": 24
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"x-ms-parameter-location": "method"

"properties": {
"apiEndpoints": {
"description": "The Media Services REST API endpoints for this resource.",
"description": "Read-only property that lists the Media Services REST API endpoints for this resource. If supplied on a PUT or PATCH, the value will be ignored.",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

documentation says that it is readOnly. However the schema definition doesn't.

"readOnly": true

"$ref": "#/definitions/MediaService"
},
"examples" : {
"id": "/subscriptions/{subscription-id}/resourceGroups/{resource-group-name}/providers/Microsoft.Media/mediaServices/{name}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above.

"$ref": "#/definitions/RegenerateKeyOutput"
},
"examples": {
"key": "{key}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above

"$ref": "#/definitions/ServiceKeys"
},
"examples":{
"primaryAuthEndpoint": "{primary-auth-endpoint}",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same as above. Make sure to cover all the review comments made in the first example

"definitions": {
"ApiEndpoint": {
"description": "The properties for a Media Services REST API endpoint.",
"readOnly": true,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ReadOnlyness can only be applied on a model Property. It cannot be applied on the entire model. Please specify this explicicitly on every property if that is the case.

}
},
"definitions": {
"ApiEndpoint": {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why have you not provided examples for model definitions?
As per the specification (once the page loads, please scroll up a little).

example | Any | A free-form property to include an example of an instance for this schema.

@BrianBlum
Copy link
Copy Markdown
Author

Submitting new PR.

@BrianBlum BrianBlum closed this Nov 10, 2016
romahamu pushed a commit to romahamu/azure-rest-api-specs that referenced this pull request Aug 26, 2019
…toragecache-2019-06-01-preview

fix typo in databox json
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.

3 participants