Skip to content

Conversation

Shivs11
Copy link
Member

@Shivs11 Shivs11 commented May 24, 2025

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.

What changed?

  • add VersionStatus inside WorkerDeploymentVersionInfo + WorkerDeploymentVersionSummary

Why?

  • right now, the UI code deduces the status of a version by checking CurrentSince, RampingSince and other attributes. This can just be simplified by adding this enum and the server doing the heavy lifting instead.

Breaking changes

  • None

Server PR

@Shivs11 Shivs11 marked this pull request as ready for review May 24, 2025 21:33
@Shivs11 Shivs11 requested review from a team as code owners May 24, 2025 21:33
Copy link
Contributor

@ShahabT ShahabT left a comment

Choose a reason for hiding this comment

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

lgtm otherwise

Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

(deferring to @Sushisource who has more background here than I do, nothing syntactically wrong from my POV)

Copy link
Member

@Sushisource Sushisource left a comment

Choose a reason for hiding this comment

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

LGTM including comment on other PR about docstrings in the list msg

@Shivs11 Shivs11 merged commit e11b55a into master May 28, 2025
7 checks passed
@Shivs11 Shivs11 deleted the ss/version_status branch May 28, 2025 16:36
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.

4 participants