Azure / azure-sdk-for-net

This repository is for active development of the Azure SDK for .NET. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/dotnet/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-net.
MIT License
5.17k stars 4.53k forks source link

You list an argument as optional and then specify in the documentation that it is required. Required parameters are not optional. #41884

Open tcrevier opened 5 months ago

tcrevier commented 5 months ago

Type of issue

Other (describe below)

Description

The docs are wrong.

Page URL

https://learn.microsoft.com/en-us/dotnet/api/azure.storage.files.shares.shareclient.-ctor?view=azure-dotnet#azure-storage-files-shares-shareclient-ctor(system-uri-azure-core-tokencredential-azure-storage-files-shares-shareclientoptions)

Content source URL

https://github.com/Azure/azure-docs-sdk-dotnet/blob/master/xml/Azure.Storage.Files.Shares/ShareClient.xml

Document Version Independent Id

46fc1a02-9475-ce4e-36b3-ece6eeaaaf4e

Article author

@azure-sdk

Metadata

jsquire commented 4 months ago

Hi @tcrevier. Thank you for reaching out and we regret that you're experiencing difficulties. Can you help us understand the specifics of what you believe is in error? We're unable to determine from context which part of the documentation you're referring to.

github-actions[bot] commented 4 months ago

Hi @tcrevier. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

tcrevier commented 4 months ago

Hi Jesse,

The constructor overload for ShareClient

ShareClient(Uri, StorageSharedKeyCredential, ShareClientOptions)

is public ShareClient (Uri shareUri, Azure.Storage.StorageSharedKeyCredential credential, Azure.Storage.Files.Shares.ShareClientOptions options = default);

and the documentation for options is

"Optional client options that define the transport pipeline policies for authentication, retries, etc., that are applied to every request."

However, the client options are not optional and default will always throw an exception. The main text of the page states :

"Also note that ShareTokenIntent https://learn.microsoft.com/en-us/dotnet/api/azure.storage.files.shares.shareclientoptions.sharetokenintent?view=azure-dotnet#azure-storage-files-shares-shareclientoptions-sharetokenintent is currently required for token authentication."

There are many many things wrong with this situation.

1) There is an optional argument (options) which is actually required. Semantically, it is not required, it will just throw an exception if you actually try to do anything. 2) The exception it throws doesn't provide any meaningful information. 3) It actually should be optional as there is only one ShareTokenIntent which you can specify. Why would anyone design software that requires you specify something where there is only one option?

For the documentation, it is obviously wrong because the documentation of the method itself says that the argument "options" is optional. It also says it is required. It is not optional. It is required. Again, you can claim that that it is semantically not.

Tom

On Sat, Feb 10, 2024 at 6:29 AM Jesse Squire @.***> wrote:

Hi @tcrevier https://github.com/tcrevier. Thank you for reaching out and we regret that you're experiencing difficulties. Can you help us understand the specifics of what you believe is in error? We're unable to determine from context which part of the documentation you're referring to.

— Reply to this email directly, view it on GitHub https://github.com/Azure/azure-sdk-for-net/issues/41884#issuecomment-1937020656, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHHRI7TV6GWGVLTPDEDYRXLYS575FAVCNFSM6AAAAABDCGYF7KVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMZXGAZDANRVGY . You are receiving this because you were mentioned.Message ID: @.***>

jsquire commented 4 months ago

@tcrevier : Thank you for the clarification. I'll route this over to the owners of the package to make the needed corrections.

github-actions[bot] commented 4 months ago

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @xgithubtriage.