[Storage] Make JSDOC more consistent and fixes #5791

joheredi · 2019-10-24T23:14:48Z

A few of the things that I went over for this PR:

Removed locale from URLs
Remove the use of [] (square brackets) for parameter and property names to avoid them to render in the docs. Also, to achieve consistency with other libraries
Fix outdated @memberof annotations from past renames
Fix missing parameter annotations
Fix wrong type annotations
Add a few missing JSDOC headers

XiaoningLiu · 2019-10-25T04:11:07Z

sdk/storage/storage-blob/src/AppendBlobClient.ts

-   * @param {AppendBlobCreateOptions} [options] Options to the Append Block Create operation.
-   * @returns {Promise<AppendBlobsCreateResponse>}
+   * @param {AppendBlobCreateOptions} options Options to the Append Block Create operation.
+   * @returns {Promise<AppendBlobCreateResponse>}


[options] means options is optional. I don't think you should remove [ and ]

@XiaoningLiu Can you elaborate on this comment? We are removing the square brackets around the parameters across the library and other libraries.

Have you been using the square brackets to indicate that a parameter is optional? Can you point us to any documentation where the square brackets are meant to be used for optional parameters?

Never mind, I found something. https://jsdoc.app/tags-param.html#optional-parameters-and-default-values

logged #5806

Thanks Ramya! I do understand the code freeze date is almost there. But please take some time to update the PR and bring squares back, I don't think there is a solid reason to against JSDoc standard about optional parameters and parameters with default values. Customers may mistakenly take optional parameters as reuqired if removing them. Please have an update, then it's good to merge.

Updated the PR and brought back the [] that were removed.

jiacfan · 2019-10-25T13:24:11Z

sdk/storage/storage-blob/src/BlobDownloadResponse.ts

@@ -322,6 +322,13 @@ export class BlobDownloadResponse implements BlobDownloadResponseModel {
    return this.originalResponse.etag;
  }

+  /**
+   * The error code


minor: missing . at end

jiacfan · 2019-10-25T13:24:27Z

sdk/storage/storage-blob/src/BlobDownloadResponse.ts

@@ -465,6 +475,12 @@ export class BlobDownloadResponse implements BlobDownloadResponseModel {
    return isNode ? this.blobDownloadStream : undefined;
  }

+  /**
+   * The HTTP response


minor: missing . at end

jiacfan · 2019-10-25T13:25:41Z

sdk/storage/storage-blob/src/ContainerClient.ts

   */
  includeDeleted?: boolean;
  /**
-   * @member {boolean} [includeMetadata] Specifies whether blob metadata be returned in the response.
+   * @member {boolean}[includeMetadata Specifies whether blob metadata be returned in the response.


typo, and please consider to bring back the [

jiacfan · 2019-10-25T13:27:47Z

sdk/storage/storage-queue/src/AccountSASSignatureValues.ts

@@ -105,7 +105,8 @@ export interface AccountSASSignatureValues {
 *
 * @see https://docs.microsoft.com/en-us/rest/api/storageservices/constructing-an-account-sas
 *
- * @param {SharedKeyCredential} sharedKeyCredential
+ * @param {AccountSASSignatureValues} accountSASSignatureValues SAS Signature values of the account
+ * @param {SharedKeyCredential} sharedKeyCredential Shared key credential


minor: missing . at end

jiacfan · 2019-10-25T13:28:41Z

sdk/storage/storage-queue/src/QueueClient.ts

@@ -929,8 +928,9 @@ export class QueueClient extends StorageClient {
   * Delete permanently removes the specified message from its queue.
   * @see https://docs.microsoft.com/en-us/rest/api/storageservices/delete-message2
   *
+   * @param {string} messageId Id of the message


minor: missing . at end, please check others if availabe

jiacfan · 2019-10-25T13:29:09Z

sdk/storage/storage-queue/src/QueueServiceClient.ts

@@ -569,7 +572,8 @@ export class QueueServiceClient extends StorageClient {
   * Deletes the specified queue permanently.
   * @see https://docs.microsoft.com/en-us/rest/api/storageservices/delete-queue3
   *
-   * @param {QueueDeleteOptions} [options] Options to Queue delete operation.
+   * @param {string} queueName name of the queue to delete


minor: . at end

jiacfan · 2019-10-25T13:29:45Z

sdk/storage/storage-queue/src/utils/utils.common.ts

@@ -141,6 +141,12 @@ function getProxyUriFromDevConnString(connectionString: string): string {
  return proxyUri;
 }

+/**
+ *
+ * @param {string} connectionString Account connection string


minor: . at end, please check other in this file as well.

…for-js into pr/joheredi/5791

ramya-rao-a · 2019-10-25T14:22:13Z

Addressed all comments, and resolved merge conflicts.

@XiaoningLiu and @jiacfan please take another look

XiaoningLiu

Looks good. Thanks Ramya!

ramya-rao-a · 2019-10-25T15:11:10Z

/azp run js - storage - ci

azure-pipelines · 2019-10-25T15:11:22Z

Azure Pipelines successfully started running 1 pipeline(s).

[Storage] Make JSDOC more consistent and fixes

bf49373

joheredi added Client This issue points to a problem in the data-plane of the library. Storage Storage Service (Queues, Blobs, Files) Docs labels Oct 24, 2019

joheredi requested review from HarshaNalluru, jeremymeng, jiacfan, vinjiang and XiaoningLiu as code owners October 24, 2019 23:14

joheredi requested a review from ramya-rao-a October 25, 2019 00:21

XiaoningLiu reviewed Oct 25, 2019

View reviewed changes

ramya-rao-a mentioned this pull request Oct 25, 2019

Use square brackets to denote optional parameters #5806

Closed

jiacfan reviewed Oct 25, 2019

View reviewed changes

ramya-rao-a added 6 commits October 25, 2019 07:05

Bring back the sqaure brackets for optional parameters

b6c2c81

Add missing [] is AppendBlobClient

88ca0bc

Bring back the sqaure brackets for optional parameters

1284fc8

Merge branch 'storage-docs' of https://github.com/joheredi/azure-sdk-…

6ed164b

…for-js into pr/joheredi/5791

Last set of cleanup

54023ee

Merge remote-tracking branch 'origin/master' into pr/joheredi/5791

c051951

XiaoningLiu approved these changes Oct 25, 2019

View reviewed changes

ramya-rao-a merged commit 81a6e76 into Azure:master Oct 25, 2019

ramya-rao-a added a commit to HarshaNalluru/azure-sdk-for-js that referenced this pull request Oct 28, 2019

Port over jsdocs updates from Azure#5791

f96282d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Storage] Make JSDOC more consistent and fixes #5791

[Storage] Make JSDOC more consistent and fixes #5791

joheredi commented Oct 24, 2019 •

edited

Loading

XiaoningLiu Oct 25, 2019

ramya-rao-a Oct 25, 2019

ramya-rao-a Oct 25, 2019

ramya-rao-a Oct 25, 2019

XiaoningLiu Oct 25, 2019 •

edited

Loading

ramya-rao-a Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019

jiacfan Oct 25, 2019 •

edited

Loading

ramya-rao-a commented Oct 25, 2019

XiaoningLiu left a comment

ramya-rao-a commented Oct 25, 2019

azure-pipelines bot commented Oct 25, 2019

[Storage] Make JSDOC more consistent and fixes #5791

[Storage] Make JSDOC more consistent and fixes #5791

Conversation

joheredi commented Oct 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XiaoningLiu Oct 25, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jiacfan Oct 25, 2019 • edited Loading

Choose a reason for hiding this comment

ramya-rao-a commented Oct 25, 2019

XiaoningLiu left a comment

Choose a reason for hiding this comment

ramya-rao-a commented Oct 25, 2019

azure-pipelines bot commented Oct 25, 2019

joheredi commented Oct 24, 2019 •

edited

Loading

XiaoningLiu Oct 25, 2019 •

edited

Loading

jiacfan Oct 25, 2019 •

edited

Loading