-
Notifications
You must be signed in to change notification settings - Fork 80
[Worker Versioning GA]: Add autoUpgrade information inside of events #665
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
Conversation
8766cbe to
aaf6f74
Compare
Sushisource
left a comment
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.
Looking good to me. I think the only other things that might be nice to add (but I won't block on):
- How does this interact with
versioning_override? I assume override comes first, but good to say. - Mentioned in my previous review: Does SDK need to care about this? If so how?
| // AutoUpgrade parent or ContinueAsNew of an AutoUpgrade run, running on the same | ||
| // deployment as the parent/previous run) | ||
| // | ||
| // Note: This field is mutually exclusive with `inherited_pinned_version. |
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.
| // Note: This field is mutually exclusive with `inherited_pinned_version. | |
| // Note: This field is mutually exclusive with `inherited_pinned_version`. |
|
Super, thanks. Putting that info about SDK into the docstring would probably be good too. |
READ BEFORE MERGING: All PRs require approval by both Server AND SDK teams before merging! This is why the number of required approvals is "2" and not "1"--two reviewers from the same team is NOT sufficient. If your PR is not approved by someone in BOTH teams, it may be summarily reverted.
Breaking changes