Skip to content

Conversation

ElizabethOkerio
Copy link
Contributor

No description provided.

/// Identifies a container for holding instance annotations. A default implementation is provided.
/// Customers can have their own implementation.
/// </summary>
public interface IODataInstanceAnnotationContainer
Copy link
Member

Choose a reason for hiding this comment

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

Actually, I don't like this interface and the methods defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you suggest it be replaced with?

/// <summary>
/// Gets or sets the annotation container to hold transient instance annotations.
/// </summary>
IODataInstanceAnnotationContainer TransientInstanceAnnotationContainer { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

where and how can customer set this interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

when they are creating the response. If there are operations in the DeltaSet that failed then you need to create DataModificationExceptions. This is the time customers will need this interface.

public abstract DeltaItemKind Kind { get; }

/// <inheritdoc/>
public IODataInstanceAnnotationContainer TransientInstanceAnnotationContainer { get => throw new NotImplementedException(); set => throw new NotImplementedException(); }
Copy link
Member

Choose a reason for hiding this comment

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

Can we mark it abstract and remove NotImplementedException()?
or at least the exception contains the message

@ElizabethOkerio ElizabethOkerio force-pushed the feature/add_data_modification_class branch from dc650e0 to d73d3af Compare May 25, 2023 07:24
@wachugamaina
Copy link

Please add a description of what this Pr does and what problem it is trying to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants