-
-
Notifications
You must be signed in to change notification settings - Fork 736
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
feat: release plan template strategy types, constraints, segments #8861
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
There's some validation to add, but that will be a separate task |
return <DefaultStrategy strategyDefinition={strategyDefinition} />; | ||
case 'flexibleRollout': | ||
return ( | ||
<MilestoneStrategyTypeFlexible |
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 one in feature strategies uses projectid and environment info, so had to take the relevant parts out and make a separate one specifically for this.
); | ||
case 'userWithId': | ||
return ( | ||
<UserWithIdStrategy |
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 along with the other UIs (Default and General) are all imported from the feature strategies. Should we make our own here or is this good enough for now?
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 is probably good enough for now, let's come back to this later.
{!BuiltInStrategies.includes(strategy.name || 'default') && ( | ||
<StyledAlertBox> | ||
<Alert severity='warning'> | ||
Custom strategies are deprecated. We recommend not |
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.
Given that custom strategies are deprecated, should we support them here at all?
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.
IMO we can do the same as we do in feature strategies for now and return to this later. But it's good to bring it up.
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.
👍 Following the philosophy of moving forward for now and coming back to clean this up later.
Adds different types of strategies and their properties to the strategy-add/edit dialog, along with constraints and segments