-
Notifications
You must be signed in to change notification settings - Fork 33
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
Add build id to workflow context #174
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need proto regen
/// be used. This value may change over the lifetime of the workflow run, but is | ||
/// deterministic and safe to use for branching. | ||
/// </remarks> | ||
public static string CurrentBuildId => Context.CurrentBuildId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for 1) not actually putting this on workflow info, and 2) respecting alphabetical order
src/Temporalio/Bridge/sdk-core
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll have to regen protos with the updated core (see https://github.com/temporalio/sdk-dotnet/#regenerating-protos, make sure it's 3.x protoc
on the PATH
). We have a CI check that makes sure our generated protos are up to date with the upstream API as seen in the core submodule and it looks like there have been additions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
fd01c91
to
74deb38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thanks!
What was changed
Why?
Part of temporalio/features#253
Checklist
Closes
How was this tested:
Added IT
Any docs updates needed?