[Azure Search] Remove inheritdoc and improve SearchCredentials documentation by bitsforthought · Pull Request #4516 · Azure/azure-sdk-for-net

bitsforthought · 2018-06-30T04:31:46Z

<inheritdoc /> is not supported by the .NET API browser. As a workaround, I'm copying documentation instead. Also adding more detail to SearchCredentials per feedback from Heidi.

Once this is in the preview SDK, I'll merge it back to the GA SDK.

FYI @HeidiSteen @Yahnoosh @mhko @jeji1101

Description

This checklist is used to make sure that common guidelines for a pull request are followed.

Please add REST spec PR link to the SDK PR
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines

Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

<inheritdoc /> is a feature of Sandcastle and is not supported by the current .NET API Browser on docs.microsoft.com. The workaround is to copy the content that would have otherwise been inherited from a base class/interface.

HeidiSteen · 2018-07-02T16:41:22Z

minor inconsistancy -- not an error, just noting it: "otherwise, false" vs "false otherwise"
Customizations/Search/Models/ExtensibleEnum.cs
Lines 73 and 88

On the short parameter descriptions, some have periods and some don't
Customizations/DataSources/DataSourceOperations.Customization.cs
Customizations/Indexers/IndexersOperations.Customization.cs
Customizations/Indexes/IndexesOperations.Customization.cs

unfamiliar HTML code -- I've never seen
There are 9 instances -- maybe do a search on the PR?
Here is one:
Customizations/Indexers/IndexersOperations.Customization.cs
Line 64
true if the indexer exists; false otherwise.

HeidiSteen · 2018-07-02T16:42:17Z

I approve the PR as long as the is valid HTML syntax.

bitsforthought · 2018-07-03T01:18:11Z

@HeidiSteen Ironically, it's the tags you didn't recognize that turned out to be the correct ones. This is technically .NET XML docs, not HTML, which can fool me sometimes. Not all HTML tags are supported by the .NET API Browser. In this case, <c></c> is supported, because it's how references or emphasis is indicated in .NET XML docs. Looking at how the existing docs are rendered on docs.microsoft.com, it appears that <b></b> is not supported, so I'm going to replace all the b's with c's.

…ssues

bitsforthought · 2018-07-03T01:25:10Z

FYI @mhko and @updixit Please review as Heidi is out of the office until next week and I'd like to merge this week. Thanks!

UD1412 · 2018-07-03T17:09:17Z

+        /// <param name='customHeaders'>
+        /// The headers that will be added to request.
+        /// </param>
+        /// <param name='cancellationToken'>


missing 'returns' tag

UD1412 · 2018-07-03T17:11:08Z

+        /// </param>
+        /// <param name='cancellationToken'>
+        /// The cancellation token.
+        /// </param>


missing a 'returns' tag

UD1412 · 2018-07-03T17:11:39Z

+        /// <param name='customHeaders'>
+        /// The headers that will be added to request.
+        /// </param>
+        /// <param name='cancellationToken'>


same as above

UD1412 · 2018-07-03T17:12:12Z

+        /// The headers that will be added to request.
+        /// </param>
+        /// <param name='cancellationToken'>
+        /// The cancellation token.


same as above.

UD1412 · 2018-07-03T22:49:30Z

+        /// <exception cref="System.ArgumentNullException">
+        /// Thrown when a required parameter is null.
+        /// </exception>
+        /// <return>


The tag is set to 'returns' at quite a few places. Please correct these.

It looks like an issue with the generated code that copied over to the custom code. It's supposed to be <returns> everywhere. I'll fix it in the custom code. In the meantime, I've filed an issue on AutoRest: https://github.com/Azure/autorest/issues/2957

bitsforthought · 2018-07-04T00:51:42Z

@UD1412 Would you mind approving using your other GitHub account, @updixit? The PR needs to be approved by an account with merge permissions to the search-preview branch.

shahabhijeet · 2018-07-13T18:13:15Z

@brjohnstmsft @HeidiSteen is this ready to be merged.
We are in middle of upgrading our repo, so if this PR is ready to be merged, this has to be approved and merged today, else we will close this PR and you will need to open a new PR against the upgraded repo.

HeidiSteen · 2018-07-13T18:23:09Z

#sign-off

updixit · 2018-07-13T19:30:09Z

#sign-off

UD1412 · 2018-07-13T19:33:21Z

#sign-off

shahabhijeet · 2018-07-17T16:41:10Z

@azuresdkci retest this please

bitsforthought added 2 commits June 29, 2018 18:42

[Azure Search] Fixing some missing documentation

6561e34

bitsforthought requested a review from HeidiSteen June 30, 2018 04:32

[Azure Search] Fixing some documentation consistency and formatting i…

ecfcb87

…ssues

bitsforthought requested review from mhko and updixit July 3, 2018 01:24

UD1412 suggested changes Jul 3, 2018

View reviewed changes

[Azure Search] Adding missing documentation to custom code

41baacc

UD1412 reviewed Jul 3, 2018

View reviewed changes

UD1412 approved these changes Jul 3, 2018

View reviewed changes

[Azure Search] Changing <return> to <returns> in custom code comments

e72924b

mhko approved these changes Jul 13, 2018

View reviewed changes

bitsforthought merged commit 457c1b1 into Azure:search-preview Jul 29, 2018

bitsforthought deleted the search-creds branch July 29, 2018 18:51

Conversation

bitsforthought commented Jun 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

HeidiSteen commented Jul 2, 2018

Uh oh!

HeidiSteen commented Jul 2, 2018

Uh oh!

bitsforthought commented Jul 3, 2018

Uh oh!

bitsforthought commented Jul 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitsforthought commented Jul 4, 2018

Uh oh!

shahabhijeet commented Jul 13, 2018

Uh oh!

HeidiSteen commented Jul 13, 2018

Uh oh!

updixit commented Jul 13, 2018

Uh oh!

UD1412 commented Jul 13, 2018

Uh oh!

shahabhijeet commented Jul 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bitsforthought commented Jun 30, 2018 •

edited

Loading