Add some content to Swagger from docs.msft.com#1549
Conversation
|
Swagger folks: Is there any reason I can't add tags or reviewers to this PR...? |
|
@itowlson @darylmsft @tamram - Please review |
matthchr
changed the title
dsgouda
left a comment
dsgouda
left a comment
There was a problem hiding this comment.
Look like non-breaking changes. LGTM apart from a minor suggestion, will approve once ready to merge.
There was a problem hiding this comment.
nit: for better documentation, consider inserting newline characters here (like so https://github.com/Azure/azure-rest-api-specs/blob/current/specification/sql/resource-manager/Microsoft.Sql/2015-05-01-preview/advisors.json#L54)
|
@erickson-doug since this is a doc related change, PTAL |
|
@matthchr are you waiting on reviews from others before merging? |
|
@dsgouda Yes, please wait for some of my colleagues to review before merging. I will let you know when we're 100% ready to merge. |
There was a problem hiding this comment.
If not specified [](start = 200, length = 16)
If what is not specified? Suggest something like: "If you do not specify a $filter clause including a startTime or endTime, these filters default to the start and end time of the last aggregation interval; that is..."
There was a problem hiding this comment.
reccomended [](start = 718, length = 11)
typo 'recommended'
There was a problem hiding this comment.
remove any successfully added tasks [](start = 778, length = 35)
this is confusingly phrased. I think we are saying "In a retry, it is most efficient to resubmit only tasks that failed to add, and to omit tasks that were successfully added on the first attempt." Or something like that?
There was a problem hiding this comment.
Yea, I like your language there, used it.
There was a problem hiding this comment.
This [](start = 351, length = 4)
Suggest tweaking this to "Reactivation will fail..." as the antecedent of 'this' is unclear
There was a problem hiding this comment.
There was a problem hiding this comment.
this [](start = 481, length = 4)
"reactivation", or "it" if you changed the previous sentence to refer to "reactivation"
There was a problem hiding this comment.
GetRemoteDesktop [](start = 298, length = 16)
Probably ought to change this to the MSDNified name e.g. "the 'connect to a compute node using Remote Desktop Protocol' API" or whatever they call it
There was a problem hiding this comment.
The MSDNified name will go away though in the new world order, so I think it's better to use a name that is more Swagger-like since I think that's going to make it easier to discover across the clients/generated markdown.
Ideally this would be some sort of "link"
There was a problem hiding this comment.
cloud service configuration [](start = 175, length = 27)
"cloudServiceConfiguration" or "created in a cloud service configuration" (that is, if you are going to separate the words then don't say you're referring to the property)
There was a problem hiding this comment.
GetRemoteLoginSettings [](start = 277, length = 22)
is there a MSDN name for this?
There was a problem hiding this comment.
There is, but again same as above it will go away when we start generating this content from Swagger I think.
There was a problem hiding this comment.
tasks [](start = 60, length = 5)
task's with an apostrophe surely?
There was a problem hiding this comment.
tasks [](start = 98, length = 5)
ditto
There was a problem hiding this comment.
S [](start = 48, length = 1)
casing is inconsistent - for job action you did not capitalise enum value descriptions
There was a problem hiding this comment.
disable - disable the job. This is equivalent to calling the disable job API, with a disableTasks value of requeue. terminate [](start = 61, length = 125)
maybe there is no better option with the present d.m.c/AutoRest situation but having a sentence after one enum value and then going straight into the name of the next is really hard to read. I don't know if embedded newlines or any other formatting will save us - if not perhaps we need something like:
"none - ....; disable - disables the job; terminate - terminates the job. The disable option is equivalent to... In the terminate option, the terminateReason..."
Still gets messy though. We need them to fix this paragraphing issue!
There was a problem hiding this comment.
I added newlines here as per @dsgouda's feedback
There was a problem hiding this comment.
If applied [](start = 422, length = 10)
minor: perhaps "If the job option is applied to a job schedule..."
There was a problem hiding this comment.
Specifies whether [](start = 20, length = 17)
Is 'specifies' safe here? I thought we preferred phrases that could be prefixed with "Gets or sets", e.g. "Whether...", but perhaps that's no longer needed?
There was a problem hiding this comment.
It is not safe here, fixed.
There was a problem hiding this comment.
Specifies [](start = 20, length = 9)
Same question about 'specifies'
There was a problem hiding this comment.
It is common to use a GUID for the id. [](start = 167, length = 38)
remove this
There was a problem hiding this comment.
Specifies [](start = 20, length = 9)
same comment
There was a problem hiding this comment.
It is common to use a GUID for the id. [](start = 167, length = 38)
remove this
There was a problem hiding this comment.
id [](start = 287, length = 2)
probably needs to be capitalised ID
There was a problem hiding this comment.
ids that can be parsed as integers. For example, a task can depend on task ids 9 to 12, but cannot depend on task ids [](start = 130, length = 117)
capitalise "IDs" now? (x3)
There was a problem hiding this comment.
"mytask" to "yourtask" [](start = 248, length = 26)
for the future, perhaps, but this is kind of inherent in the type spec. I think we reworked this in the C# docs to illustrate how an integer range mapped to stringy IDs - perhaps we should review and decide which we prefer. Not needed for this port anyway
There was a problem hiding this comment.
Values [](start = 26, length = 6)
the order of these seems pretty random. Might be good to try to organise them a bit more given the size of the enum (though I'm not sure whether there is an order that people would find predictable, other than plain alphabetical which may not be all that helpful)
There was a problem hiding this comment.
They are described in the order they are listed in the enum in the spec (so from 0 to N). I think that order makes the most sense, and I can't change that order because it's technically a breaking change.
There was a problem hiding this comment.
Running tasks [](start = 1217, length = 13)
this becomes confusing especially with the flattening of the paragraphs. Perhaps "Tasks which were running on the node when it was pre-empted..."?
There was a problem hiding this comment.
[](start = 830, length = 1)
needs a dash
There was a problem hiding this comment.
Note [](start = 26, length = 4)
this was in a title somewhere else - I think it is probably better in the description
There was a problem hiding this comment.
I can't find any places where this is in a title
There was a problem hiding this comment.
delete [](start = 667, length = 6)
operation?
matthchr
force-pushed
the
feature/batch-swagger-docs
branch
from
2fec71b to
069d9ba
Compare
matthchr
changed the title
|
@dsgouda If the linter looks good you can feel free to merge this now |
- This is in preparation for using the Swagger specification as the documentation
authority that generates docs.msft.com documentation.
matthchr
force-pushed
the
feature/batch-swagger-docs
branch
from
069d9ba to
8e10c31
Compare
|
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: AutoRest Linter Guidelines | AutoRest Linter Issues Send feedback and make AutoRest Linter Azure Bot smarter day by day! Thanks for your co-operation. |
1 similar comment
|
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: AutoRest Linter Guidelines | AutoRest Linter Issues Send feedback and make AutoRest Linter Azure Bot smarter day by day! Thanks for your co-operation. |
|
No modification for AutorestCI/azure-sdk-for-node |
|
No modification for AutorestCI/azure-sdk-for-ruby |
This reverts commit b955458.
* Revert "[Event Grid] Event grid C# code generation section. (#1561)" This reverts commit 461a494. * Revert "Bug Fix when linter runs on json file without being included in tag (#1560)" This reverts commit d6bc117. * Revert "Remove databaseName uri param from Databases_Import op. (#1558)" This reverts commit 69d0a5d. * Revert "Added 200 response for event grid event subscription delete operation. (#1555)" This reverts commit ad55af7. * Revert "Add some content to Swagger from docs.msft.com (#1549)" This reverts commit b955458. * Revert "[Azure Analysis Services] Add gateway info to version 0714 and version 0801 (#1526)" This reverts commit bf407b7. * Revert "Copied service endpoints specs to 2017-08-01 (#1548)" This reverts commit 64c905a. * Revert "Removed `x-ms-pageable` from Network Interface's `GetEffectiveRouteTable` and `ListEffectiveNetworkSecurityGroups` methods (#1547)" This reverts commit da940d5. * Revert "Added service endpoints APIs (#1533)" This reverts commit f69dc64. * Revert "Added support for ECC to Key Vault (#1538)" This reverts commit 4a9084f.
change the folder name to 2020-06-10-preview
6 participants
authority that generates docs.msft.com documentation.
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
api-versionin the path should match theapi-versionin the spec).Quality of Swagger