Skip to content

Adding Swagger for Domain renewal API#2155

Merged
jianghaolu merged 8 commits into
Azure:masterfrom
mn263:current
Jan 17, 2018
Merged

Adding Swagger for Domain renewal API#2155
jianghaolu merged 8 commits into
Azure:masterfrom
mn263:current

Conversation

@mn263
Copy link
Copy Markdown
Contributor

@mn263 mn263 commented Dec 15, 2017

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

"204": {
"description": "Domain does not exist."
},
"400": {
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.

All the negative response status code descriptions MUST be defined in the description of the "default" status code (if the intention is for documentation purpose only). Example: "description": "'404' - 'Not found'.\n'400' - 'Bad request'.\n'500': 'Internal Server Error'."'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean. Do you mean just changing the description from something like "Malformed domain renewal request." to "'400' - 'Bad request'." ?

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.

If you list a response code like this it will be treated as a valid response code. Thus you must not list error response codes but put all of them in meaningful descriptions in status code "default". E.g.:

"200": {
"description": "Domain renewal was successful."
},
"202": {
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.

Is this a long running operation? Please add "x-ms-long-running-operation": true if so.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it's not.

],
"summary": "Renew a domain.",
"description": "Renew a domain.",
"operationId": "Domains_Renew",
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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done with latest commit.

@marstr marstr changed the base branch from current to master December 28, 2017 17:54
@azuresdkciprbot
Copy link
Copy Markdown

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/web/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 45
After the PR: Warning(s): 0 Error(s): 45

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

},
"responses": {
"200": {
"body": {}
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.

Remove "body": {} since the operation doesn't return a body

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

"domainName": "example.com"
},
"responses": {
"200": {
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.

Please also add 202 and 204 here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@azuresdkciprbot
Copy link
Copy Markdown

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/web/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 45
After the PR: Warning(s): 0 Error(s): 45

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@mn263 mn263 closed this Jan 17, 2018
@AutorestCI
Copy link
Copy Markdown

Was unable to find SDK Azure/azure-sdk-for-python PR for this closed PR.

@mn263 mn263 reopened this Jan 17, 2018
@jianghaolu jianghaolu merged commit a70ffe7 into Azure:master Jan 17, 2018
@AutorestCI
Copy link
Copy Markdown

Was unable to find SDK Azure/azure-sdk-for-python PR for this closed PR.

@AutorestCI
Copy link
Copy Markdown

@AutorestCI
Copy link
Copy Markdown

@AutorestCI
Copy link
Copy Markdown

No modification for AutorestCI/azure-sdk-for-python

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.

4 participants