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

.Net: Process Framework: Simplify Event Emission in Process Steps #9560

Conversation

joslat
Copy link
Contributor

@joslat joslat commented Nov 5, 2024

Motivation and Context

Fixes: #9510

Process Framework: Simplify Event Emission in Process Steps
I would like to propose a simplification of the event emission mechanism within the Process Framework, specifically in process steps. Currently, emitting events involves multiple method calls and boilerplate code, which can make the codebase harder to read and maintain.

Description

I created an overload of EmitEventAsync with simplified signature to reduce developer cognitive load, updated some tests to verify on build.

If decided to update the signature completely (replace it) I can update all the examples and tests.

Contribution Checklist

…e developer cognitive load, updated some tests to verify on build.

If decided to update the signature completely (replace it) I can update all the examples and tests.
@joslat joslat requested a review from a team as a code owner November 5, 2024 19:38
@markwallace-microsoft markwallace-microsoft added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel labels Nov 5, 2024
@github-actions github-actions bot changed the title Process Framework: Simplify Event Emission in Process Steps .Net: Process Framework: Simplify Event Emission in Process Steps Nov 5, 2024
@crickman crickman enabled auto-merge November 5, 2024 19:57
@crickman crickman added PR: ready for review All feedback addressed, ready for reviews experimental Associated with an experimental feature enhancement processes labels Nov 5, 2024
auto-merge was automatically disabled November 5, 2024 20:08

Head branch was pushed to by a user without write access

@esttenorio
Copy link
Contributor

esttenorio commented Nov 5, 2024

@joslat Not blocking - since this is an additional way of emitting events perhaps some of the steps in the Getting Started With Processes should have this so people are aware there are multiple ways of achieving this.

It is more likely for a person trying out Processes for the first time to check out the Getting Started samples than to check the integration tests for examples of usage

@crickman
Copy link
Contributor

crickman commented Nov 5, 2024

Not blocking - since this is an additional way of emitting events perhaps some of the steps in the Getting Started With Processes should have this so people are aware there are multiple ways of achieving this.

It is more likely for a person trying out Processes for the first time to check out the Getting Started samples than to check the integration tests for examples of usage

Step01 would be an easy update.

@joslat
Copy link
Contributor Author

joslat commented Nov 5, 2024

@joslat Not blocking - since this is an additional way of emitting events perhaps some of the steps in the Getting Started With Processes should have this so people are aware there are multiple ways of achieving this.

It is more likely for a person trying out Processes for the first time to check out the Getting Started samples than to check the integration tests for examples of usage

Hi @esttenorio I was thinking of providing a super-simple example with 3-4 super simple steps like this one here: https://github.com/joslat/semantic-kernel/tree/joslat-process-framework-graph-edges/dotnet/samples/ProcessDemoGraphNodesEdges for example :)

@crickman crickman added this pull request to the merge queue Nov 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2024
@crickman crickman added this pull request to the merge queue Nov 5, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2024
@crickman crickman enabled auto-merge November 5, 2024 21:39
@crickman crickman added this pull request to the merge queue Nov 5, 2024
Merged via the queue into microsoft:main with commit 3332079 Nov 5, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement experimental Associated with an experimental feature kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready for review All feedback addressed, ready for reviews processes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Process Framework: Simplify Event Emission in Process Steps
5 participants