Skip to content

Conversation

@tconley1428
Copy link
Contributor

@tconley1428 tconley1428 commented Oct 24, 2025

What was changed

Added ITemporalClientPlugin and ITemporalWorkerPlugin to provide configurability to their respective objects. SimplePlugin was added to make it easier to implement plugins consistently across objects.

Why?

It can be difficult to provide consistent and easy methods of configuring temporal objects.

Checklist

  1. Closes [Feature Request] Plugin support #507 #plu

  2. How was this tested:
    New set of plugin tests.

  3. Any docs updates needed?
    Plugin documentation in general is pending.

@tconley1428 tconley1428 changed the title Plugin/initial Initial implementation of plugins Oct 29, 2025
@tconley1428 tconley1428 marked this pull request as ready for review October 29, 2025 18:40
@tconley1428 tconley1428 requested a review from a team as a code owner October 29, 2025 18:40
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

All looks good to me, but I'm a bit biased because I was involved in the coding. @maciejdudko - can you take a look at this review?

Copy link
Contributor

@jmaeagle99 jmaeagle99 left a comment

Choose a reason for hiding this comment

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

Wrote some thoughts down, but feel free to take or leave any of them.

/// <summary>
/// Gets the constant value for the required option.
/// </summary>
public T? Constant { get; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this nullable? Would it not be sufficient to declare the usage of the SimplyPluginOption<T> as nullable itself? The only scenario I can think of where you might need this is if you want to say that this option doesn't have a non-null default value and you want to have the configurable callback; this feels like a simplified design of the options pattern in .NET where any number of consumers of the library could implement IConfigureOptions<T> and IPostConfigureOptions<T> for declaring and registering callbacks for mutating options before a consume of the options gets the final value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That does sound similar. It's trying to mimic what I did in the other languages with either taking a value or a function that produces it. The options pattern seems to be doing the same thing.

Copy link
Member

@cretz cretz Nov 10, 2025

Choose a reason for hiding this comment

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

In this side of the SDK, we don't make any assumptions about IConfigureOptions and such, so we can only provide naive forms of callback-based options mutations. So this is effectively a "constant or function" mutually exclusive option value. Technically we could have made a whole abstract base class sum type thing, but this class should be rarely used anyways.

@tconley1428 tconley1428 merged commit 5546879 into main Nov 11, 2025
11 checks passed
@tconley1428 tconley1428 deleted the plugin/initial branch November 11, 2025 18:40
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.

[Feature Request] Plugin support

5 participants