Skip to content

EDU-3824: Add best practice advice to Typescript Versioning #3303

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

Draft
wants to merge 15 commits into
base: main
Choose a base branch
from

Conversation

fairlydurable
Copy link
Contributor

Source of truth/Author: Grant Smith

@fairlydurable fairlydurable requested a review from a team as a code owner January 22, 2025 16:22
@fairlydurable fairlydurable added waiting-on-tech-review PR is blocked and waiting for tech review from Subject Matter Experts tracking-internally labels Jan 22, 2025
@GSmithApps GSmithApps added the work-in-progress This PR is not yet ready for technical or team review. Ideally it should be converted to a draft. label Jan 25, 2025
@GSmithApps GSmithApps marked this pull request as draft January 25, 2025 20:28
@GSmithApps
Copy link
Contributor

After experimenting more in my local, I see that I'm missing something. I'll circle back when i'm more solid

@GSmithApps
Copy link
Contributor

GSmithApps commented Jan 26, 2025

Okay yeah I have a pretty good grasp of what's going on now -- at least in the python SDK. I'll need to revise the writuep and test in other SDKs, but I made a little youtube series deep diving into it. we can maybe put that in the docs too 👍

@GSmithApps
Copy link
Contributor

Yep, I looked into TS and Dotnet as well. Here's the writeup. I'll work on getting it to a point that can be merged etc

@@ -108,6 +108,90 @@ Implementing patching involves three steps:
2. Remove the old code and apply [DeprecatePatch](https://dotnet.temporal.io/api/Temporalio.Workflows.Workflow.html#Temporalio_Workflows_Workflow_DeprecatePatch_System_String_).
3. Once you're confident that all old Workflows have finished executing, remove `DeprecatePatch`.

#### Overview

The following sample shows how the `patched()` function behaves, providing explanations at each stage of the patching flow:
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a line about defining the version conditions in descending order?

Copy link
Member

@kevinawoo kevinawoo Jan 30, 2025

Choose a reason for hiding this comment

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

Something like

The best practice is to write your version conditionals in descending order so that newly executed workflows will set their marker as the latest version.

The default value for the version is "" (empty string)

@fairlydurable fairlydurable added cross-team This issue or PR was submitted from within Temporal status-check-please Author, would you please let us know what the state of this PR is. Thank you. and removed tracking-internally labels Feb 27, 2025
@fairlydurable
Copy link
Contributor Author

@GSmithApps Status check on this one? Thanks!

@GSmithApps
Copy link
Contributor

GSmithApps commented Mar 5, 2025

@fairlydurable fairlydurable added waiting-on-docs-review Work is complete and ticket is tech reviewed. Blocked by needing team review for non-trivial change and removed status-check-please Author, would you please let us know what the state of this PR is. Thank you. waiting-on-tech-review PR is blocked and waiting for tech review from Subject Matter Experts labels Mar 11, 2025
@fairlydurable fairlydurable deleted the EDU-3824 branch April 11, 2025 18:14
@fairlydurable fairlydurable restored the EDU-3824 branch April 14, 2025 21:11
@fairlydurable fairlydurable reopened this Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cross-team This issue or PR was submitted from within Temporal waiting-on-docs-review Work is complete and ticket is tech reviewed. Blocked by needing team review for non-trivial change work-in-progress This PR is not yet ready for technical or team review. Ideally it should be converted to a draft.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants