Skip to content
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

[Storage] [DataMovement] Put EditorBrowsable(Never) for not using interfaces #47939

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

amnguye
Copy link
Member

@amnguye amnguye commented Jan 22, 2025

Added [EditorBrowsable(EditorBrowsableState.Never)] for DataMovement interfaces that are not meant to be used publicly.

@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Jan 22, 2025
@azure-sdk
Copy link
Collaborator

azure-sdk commented Jan 22, 2025

API change check

API changes are not detected in this pull request.

protected internal abstract bool IsContainer { get; }

/// <summary>
/// Gets the Uri of the Storage Resource.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public abstract Uri Uri { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Uri may be useful to leave so customers are able to identity their resource objects if they need to. I also sort of feel this way about IsContainer but we marked that as protected internal meaning we didn't want customers to use it so ehh

using System.IO;

namespace Azure.Storage.DataMovement
{
/// <summary>
/// Abstract class for checkpoint data related to a specific resource type.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Did we check if marking a class as EB(never) also marks its properties?

@@ -13,46 +14,55 @@ public class TransferProperties
/// <summary>
/// Contains the transfer ID which to rehydrate the StorageResource from.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe since we are handing these back to customers when they call GetTransfers we should have some info visible. Maybe TransferId, SourceUri, and DestinationUri?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants