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

MVP .NET referential feature & inter-tech how-to documentation #2769

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

flossypurse
Copy link
Member

No description provided.

@flossypurse flossypurse requested a review from a team as a code owner April 22, 2024 21:06
@flossypurse flossypurse marked this pull request as draft April 22, 2024 21:06
@flossypurse flossypurse changed the title Scaffolding .net pages Scaffolding .NET SDK pages Apr 22, 2024
Comment on lines +52 to +54
##### Cancel

Canceling a Workflow provides a graceful way to stop Workflow Execution.
Copy link
Member

Choose a reason for hiding this comment

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

This section and what's expected to go in it is unclear to me

Comment on lines 31 to 51
## Payload conversion

<!-- CONTENT-->

### Conversion sequence {#conversion-sequence}

<!-- CONTENT-->

### Custom Payload Converter {#custom-payload-converter}

**How to use a custom Payload Converter with the .NET SDK.**

<!-- CONTENT-->

**List of default Data Converter supports converting multiple types**

<!-- CONTENT-->

### Custom Type Data Conversion

<!-- CONTENT-->
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

Only the payload codec is related to encryption, none of the other sections are related to encryption in any way and are used by users for type conversion customization, not encryption.


## Request Cancellation {#request-cancellation}

**How to request Cancellation of a Workflow and Activities in Go.**
Copy link
Member

Choose a reason for hiding this comment

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

Requesting cancellation of a workflow is a different concept than requesting cancellation of an activity, so I am splitting these


<!-- CONTENT-->

### How to terminate a Workflow Execution in .NET {#terminate-a-workflow-execution}
Copy link
Member

Choose a reason for hiding this comment

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

The heading depth is a bit confusing here


Canceling a Workflow provides a graceful way to stop Workflow Execution.

##### Terminate
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

This file and entire section is "How to cancel a Workflow Execution and it's (sic) Activities using the .NET SDK.". Termination shouldn't be buried in cancellation documentation.

- overview
---

This pages shows the following:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This pages shows the following:
This page shows the following:

id: cancellation
title: Cancellation - .NET SDK feature guide
sidebar_label: Cancellation
description: How to cancel a Workflow Execution and it's Activities using the .NET SDK.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description: How to cancel a Workflow Execution and it's Activities using the .NET SDK.
description: How to cancel a Workflow Execution and its Activities using the .NET SDK.


<!-- CONTENT-->

## Start a Workflow {#start-worklow}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Start a Workflow {#start-worklow}
## Start a Workflow {#start-workflow}

@@ -0,0 +1,19 @@
---
id: continue-as-new
title: How to Continue-As-New in Dotnet
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
title: How to Continue-As-New in Dotnet
title: How to Continue-As-New in .NET

Inconsistent

Comment on lines +3 to +5
title: Date Encryption
sidebar_label: Date encryption
description: Date encryption
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

Misspelled "Data"

@@ -0,0 +1,24 @@
---
id: child-workflows
title: Child Workflows in .NET
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent with whether you suffix titles with " - .NET SDK feature guide"


<!-- CONTENT-->

## Skip time in tests {#testing-skip-time}
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

I don't think this should be here, I think it should be with the testing documentation. It's not about durable timers a user may create, it's also about timeouts and other things.

- failure-detection
---

## Failure Detection {#failure-detection}
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

What is expected to be put under this heading? The next heading is not a child. Is there existing SDK documentation you can point to as a guide so we can be consistent here?


## Failure Detection {#failure-detection}

## Workflow timeouts {#workflow-timeouts}
Copy link
Member

@cretz cretz May 1, 2024

Choose a reason for hiding this comment

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

The title of this document is failure detection but we often don't consider timeouts the same as failures. Also, not sure these general purpose options are about failure detections. I think they deserve to be next to other workflow options.


<!-- CONTENT-->

### How to terminate a Workflow Execution in .NET {#terminate-a-workflow-execution}
Copy link
Member

Choose a reason for hiding this comment

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

The "in .NET" suffix is a bit inconsistent here with other headings

- heartbeat-timeout
---

## How to Heartbeat an Activity {#Activity-heartbeats}
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent anchor capitalization

@@ -0,0 +1,109 @@
---
id: message-passing
title: Application message passing - .NET SDK feature guide
Copy link
Member

Choose a reason for hiding this comment

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

"Workflow interaction" I think is a better name for these concepts than "Application message passing"

@@ -0,0 +1,26 @@
---
id: heartbeat
Copy link
Member

Choose a reason for hiding this comment

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

ID inconsistent with file name


<!-- CONTENT-->

## How to interrupt a Workflow Execution {#interrupt-a-workflow-execution}
Copy link
Member

Choose a reason for hiding this comment

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

We are inconsistent with whether we put "How to" in the heading title (e.g. it's "Request Cancellation" not "How to request Cancellation")

@cretz
Copy link
Member

cretz commented May 1, 2024

Note, work on the .NET docs has already started off the current branch, so any changes to this branch may not be reflected. Instead, take the review comments as changes that need to be made when cleaning up the submitted .NET docs.


<!-- CONTENT-->

### Custom Type Data Conversion
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Custom Type Data Conversion
### Custom type Data Conversion

Unsure if "Type" should be capitalized in this case


<!-- CONTENT-->

## Payload conversion
Copy link
Member

Choose a reason for hiding this comment

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

Conversion not capitalized here but it is with Data Conversion below


<!-- CONTENT-->

## How to set Activity timeouts {#activity-timeouts}
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent headings with workflow timeouts (this is prefixed with "How to set" but "Workflow timeouts" section is not)

- feature guidance
---

<!-- .NET SDK feature guidance landing page-->
Copy link
Member

Choose a reason for hiding this comment

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

This is the first ever feature guidance landing page, so unsure what to add here. I am assuming I follow the dev-guide landing pages?

- updates
---

<!-- What is on this page-->
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent with whether you provide this or expect it to be provided. I would expect this could have already been filled in the way it is in some other pages.

Comment on lines 46 to 48
**List of metrics capable of being emitted**

<!-- CONTENT-->
Copy link
Member

Choose a reason for hiding this comment

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

This appears to be brand new for .NET. I don't think .NET is where we should introduce this new bold heading, it should be done consistently.


<!-- CONTENT-->

### How to remove a Search Attribute from a Workflow {#remove-search-attribute}
Copy link
Member

Choose a reason for hiding this comment

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

This section is not needed, upsert includes removal


### How to upsert Search Attributes {#upsert-search-attributes}

**How to upsert Search Attributes to add or update Search Attributes from within Workflow code**
Copy link
Member

Choose a reason for hiding this comment

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

This is the same as the next part, there's no difference between the two

- dotnet
- schedules
---

Copy link
Member

Choose a reason for hiding this comment

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

We are inconsistent with whether the top of the page has a summary of what's on the page


<!-- CONTENT-->

## How to use Temporal Cron Jobs {#temporal-cron-jobs}
Copy link
Member

Choose a reason for hiding this comment

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

I think this section should be removed

Comment on lines 61 to 77
## How to use Worker Versioning in .NET {#worker-versioning}

<!-- CONTENT-->

### Assign a Build ID to your Worker

<!-- CONTENT-->

### Specify versions for Commands

**How to specify versioning intent**

<!-- CONTENT-->

**How to use the latest default version of an Activity**

<!-- CONTENT-->
Copy link
Member

Choose a reason for hiding this comment

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

I don't believe these sections should be added at this time knowing they are already obsolete pending new versioning implementation


## Introduction to Versioning

## How to use the .NET SDK Patching API {#.NET-sdk-patching-api}
Copy link
Member

Choose a reason for hiding this comment

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

Inconsistent use of the runtime name in the anchor

@cretz cretz mentioned this pull request May 3, 2024
Co-authored-by: Cully Wakelin <cully@temporal.io>
@flossypurse flossypurse marked this pull request as ready for review May 6, 2024 17:50
@flossypurse flossypurse changed the title Scaffolding .NET SDK pages .MVP NET referential feature & inter-tech how-to documentation May 8, 2024
@flossypurse flossypurse changed the title .MVP NET referential feature & inter-tech how-to documentation MVP .NET referential feature & inter-tech how-to documentation May 9, 2024
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.

None yet

4 participants