Skip to content

Add initial ITaskEntity abstractions#165

Merged
jviau merged 5 commits intomicrosoft:feature/entitiesfrom
jviau:entities
Jul 27, 2023
Merged

Add initial ITaskEntity abstractions#165
jviau merged 5 commits intomicrosoft:feature/entitiesfrom
jviau:entities

Conversation

@jviau
Copy link
Member

@jviau jviau commented Jul 10, 2023

resolves #163

This PR adds abstractions and supporting types for authoring durable entities:

  • ITaskEntity
  • TaskEntityOperation
  • TaskEntityContext

Copy link
Member

@sebastianburckhardt sebastianburckhardt left a comment

Choose a reason for hiding this comment

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

looks good to me. I have one suggestion that seems relevant (allow empty string for entity keys) and beyond that just a few minor comments/suggestions.

/// <param name="id">The entity to signal.</param>
/// <param name="operationName">The operation name.</param>
/// <param name="input">The operation input.</param>
/// <param name="options">The options to signal with.</param>

Choose a reason for hiding this comment

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

would it make sense to mention here that it is possible to pass a DateTimeOffset directly (thanks to the implicit conversion)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would prefer to leave that for discovery by looking at the SignalEntityOptions type itself. Purely because if I add it here, I should be adding it to all the other usages of it for consistency. And that seems like a lot of extra comments for little value.

Copy link
Member

@sebastianburckhardt sebastianburckhardt left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jviau jviau merged commit 244c68d into microsoft:feature/entities Jul 27, 2023
@jviau jviau deleted the entities branch July 27, 2023 16:57
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.

2 participants