-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix see href links - Part 1 #12883
fix see href links - Part 1 #12883
Conversation
@@ -14,7 +14,7 @@ public class KeyVaultAccessControlClientOptions : ClientOptions | |||
/// <summary> | |||
/// The latest service version supported by this client library. | |||
/// For more information, see | |||
/// <see href="https://docs.microsoft.com/en-us/rest/api/keyvault/key-vault-versions"/>. | |||
/// <see href="https://docs.microsoft.com/rest/api/keyvault/key-vault-versions">Key Vault versions</see>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wasn't sure if I should just remove this or if there's a better replacement. There's no such article.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Azure Cognitive Search changes look good to me. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Key Vault and Search changes LGTM. Thank you! I'll ask the KV PM about that link.
@@ -14,7 +14,7 @@ public class KeyVaultAccessControlClientOptions : ClientOptions | |||
/// <summary> | |||
/// The latest service version supported by this client library. | |||
/// For more information, see | |||
/// <see href="https://docs.microsoft.com/en-us/rest/api/keyvault/key-vault-versions"/>. | |||
/// <see href="https://docs.microsoft.com/rest/api/keyvault/key-vault-versions">Key Vault versions</see>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AppConfig LGTM - thanks!
@mairaw we're about to GA Search. Is this PR ready, or should I make the same changes separately so we can get that in ASAP? /cc @tf-msft |
I think you were going to check with the owner if there was a better replacement there @heaths but the changes should be good as-is. This could be improved later on. |
They are working to restore that link. I requested they use the same one. There's nothing else to which it could point, so I think this is fine to merge. Do you have permissions to do so, or would you like me to? |
I don’t have permissions |
Thanks, @mairaw! |
While browsing the docs, I've noticed several missing links such as
https://docs.microsoft.com/en-us/dotnet/api/azure.storage.files.shares.sharefileclient.-ctor?view=azure-dotnet#Azure_Storage_Files_Shares_ShareFileClient__ctor_System_String_System_String_System_String_
When I was working on dotnet-api-docs, the expected syntax includes the link text as well (which also helps with SEO). I also removed the en-us locale from links that contained that.