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

V1.5.1 #30

Merged
merged 6 commits into from
Dec 17, 2024
Merged

V1.5.1 #30

merged 6 commits into from
Dec 17, 2024

Conversation

kbilsted
Copy link
Owner

No description provided.

Copy link

@hoji-ai hoji-ai bot left a comment

Choose a reason for hiding this comment

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

Summary

  • Updated code lines, test lines in badges in README file.
  • Improved explanation and clarified instructions in README for defining workflow steps and design philosophy.
  • Updated package versions in various .csproj files.
  • Added Explicit attribute to PerformanceTests to mark them slow.
  • Replaced some method calls with asynchronous pattern using Task.FromResult.
  • Removed unnecessary transaction initialization in WorkflowRuntimeData.

Review

Reviewed changes in documentation and code updates. Noted a series of typos and grammatical issues in the README.md files that need addressing for clarity and professionalism. Majorly focused on improving the readability and understanding of the workflow concept explanations and instructions.

Checked the usage of Task.FromResult() in test codes and recommended where asynchronous operations are involved.

Advised consolidating inconsistent statements regarding naming conventions for workflow steps.

@@ -1,11 +1,10 @@
# Micro Workflow .net
<!--start-->
[![Stats](https://img.shields.io/badge/Code_lines-1,7_K-ff69b4.svg)]()
[![Stats](https://img.shields.io/badge/Code_lines-1,8_K-ff69b4.svg)]()
Copy link

Choose a reason for hiding this comment

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

Please ensure to update the Test and Doc lines if there were actual changes in the count reflect the accurate statistics.


**Simplicity**
* We model only the steps in a workflow, *not* the transitions between them
* This greatly *simplify* the model, the versioning of flows and steps
* enable you to use reusable code blocks for determining a transition
* It is *easy* to embed it directly into your solutions to improve resiliance or use as a stand-alone workflow
Copy link

Choose a reason for hiding this comment

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

Typo: "resiliance" should be spelled "resilience" for clarity.


**Simplicity**
* We model only the steps in a workflow, *not* the transitions between them
* This greatly *simplify* the model, the versioning of flows and steps
* enable you to use reusable code blocks for determining a transition
* It is *easy* to embed it directly into your solutions to improve resiliance or use as a stand-alone workflow
Copy link

Choose a reason for hiding this comment

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

The phrase "to improve resiliance or use as a stand-alone workflow" is slightly misleading. Consider clarifying that micro-workflow can improve resilience and can also be used as a stand-alone workflow.

* It is *easy* to embed it directly into your solutions to improve resiliance or use as a stand-alone workflow
* This greatly *simplifies* the model, especially when dealing with change and versioning
* It enables you to use reusable code blocks for determining a transition
* It is *easy* to embed micro workflow directly into your solutions to improve resiliance or use as a stand-alone workflow
Copy link

Choose a reason for hiding this comment

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

Typo: "debugable" should be spelled "debuggable."

* It is *easy* to embed it directly into your solutions to improve resiliance or use as a stand-alone workflow
* This greatly *simplifies* the model, especially when dealing with change and versioning
* It enables you to use reusable code blocks for determining a transition
* It is *easy* to embed micro workflow directly into your solutions to improve resiliance or use as a stand-alone workflow
Copy link

Choose a reason for hiding this comment

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

Typo: "testable" should be spelled "testable."

@@ -61,11 +65,11 @@ Supported scalabilities

# 3. Getting started

To define a workflow with the two steps `FetchData` (which fetches some data), and `AnalyzeWords` (that analyzes the data), we implement interface `IStepImplementation` twice.
Copy link

Choose a reason for hiding this comment

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

There is an inconsistency as "There are no restrictions on the names of steps" and "Two workflow steps cannot have the same name". Consider consolidating these statements for better clarity.

@@ -74,13 +74,14 @@ public void When_AddStepIfNotExists_two_identical_singleton_steps_Then_insert_fi
var name = helper.RndName;
Copy link

Choose a reason for hiding this comment

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

The change here from a single chain statement to a variable assignment and chain statement, improves readability and debugging, however, if id is expected to be reused later, it's more appropriate. Be sure this change aligns with the rest of the code.

@@ -65,7 +65,7 @@ public async Task<ExecutionResult> ExecuteAsync(Step step)

// ... stuff

return step.Done();
Copy link

Choose a reason for hiding this comment

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

By using Task.FromResult, you're ensuring the code is asynchronous but it can be considered redundant if the operation is not truly asynchronous, potentially confusing future maintainers.


Design philosophy

Copy link

Choose a reason for hiding this comment

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

Consider harmonizing bullet formatting where past bullet points end with periods, for uniformity.

<!--end-->


<p align="center"> <img src="doc/microworkflow.webp" alt="logo"></p>

Copy link

Choose a reason for hiding this comment

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

Typo: "embedable" should be spelled "embeddable."

@kbilsted kbilsted merged commit 03ec21f into master Dec 17, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants