Skip to content

Conversation

@jakejscott
Copy link
Contributor

  1. Closes Sample request: DSL #19

  2. How was this tested:
    Manually + tests.

  3. Any docs updates needed?
    README updated

@jakejscott jakejscott requested a review from a team as a code owner January 4, 2026 03:34
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.

Looks great, nothing blocking necessarily, just pedantic code structure things

required public ParallelBranches Parallel { get; init; }
}

public record DslInput
Copy link
Member

@cretz cretz Jan 5, 2026

Choose a reason for hiding this comment

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

Pedantic, but I think this should be the top-level type (and file name), and it should have a static Parse() and all of these other records should be inner classes of it.

Comment on lines 9 to 14
private Dictionary<string, object> variables = new();

[WorkflowRun]
public async Task<Dictionary<string, object>> RunAsync(DslInput input)
{
variables = new Dictionary<string, object>(input.Variables);
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend using [WorkflowInit] for this. E.g.

private readonly Dictionary<string, object> variables;

[WorkflowInit]
public DslWorkflow(DslInput input) => variables = input.Variables;

case ActivityStatement activityStmt:
// Invoke activity loading arguments from variables and optionally storing result as a variable
var args = activityStmt.Activity.Arguments
.Select(argName => variables.TryGetValue(argName, out var value) ? value : string.Empty)
Copy link
Member

Choose a reason for hiding this comment

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

Would be ok using index accessor and letting this just throw on key not found to keep logic simpler

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.

Thanks!

@cretz cretz merged commit 4a24bce into temporalio:main Jan 7, 2026
10 of 11 checks passed
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.

Sample request: DSL

2 participants