-
Notifications
You must be signed in to change notification settings - Fork 66
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
Rename Update Requested to Update Admitted #387
Rename Update Requested to Update Admitted #387
Conversation
@@ -37,7 +37,7 @@ $(PROTO_OUT): | |||
mkdir $(PROTO_OUT) | |||
|
|||
##### Compile proto files for go ##### | |||
grpc: buf-lint api-linter buf-breaking clean go-grpc fix-path | |||
grpc: buf-lint api-linter clean go-grpc fix-path |
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.
this will be re-enabled after merge
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!
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.
In the future, for larger bits of work like this, I wonder if we should be working in a branch until implementation is complete (similar to what versioning is doing).
## What changed? Rename field on internal update proto. ## Why? Consistency with the "requested" => "admitted" renaming started in temporalio/api#387 ## Potential risks None ## Is hotfix candidate? No
What changed?
Renamed "Update Requested" to "Update Admitted".
Why?
After some discussion, we came to the conclusion that the name should align with Update's wait stage in order to simplify the mental model for the user. "Requested" would be yet another concept, but "Admitted" (wait stage) already exists and should be used instead.
Breaking changes
Yes, but the renamed types are not used by the Server yet.
They are present in:
And subsequent PRs will adjust these accordingly.
Server PR
Not needed; these types are not used in Server yet.