chore: GA (remove flag) for Slack App integration #4765
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git 鈫楋笌 |
| @@ -9,7 +9,7 @@ export const addonDefinitionSchema = joi.object().keys({ | |||
| documentationUrl: joi.string().uri({ scheme: [/https?/] }), | |||
| description: joi.string().allow(''), | |||
| howTo: joi.string().optional().allow(''), | |||
| deprecated: joi.boolean().optional().default(false), | |||
| deprecated: joi.string().optional().allow(''), | |||
There was a problem hiding this comment.
Is this change needed? Sounds like deprecated should be a boolean. In OpenAPI there's deprecated + deprecatedDescription IIRC
There was a problem hiding this comment.
This fixes and adapts the addon schema to how the deprecated property really works right now: It acts as both the boolean that tells us whether an addon/integration is deprecated and the message why.
This feels pretty clear to me, as you won't specify a deprecation property for non-deprecated addons. But if you feel it's clearer to have 2 separate properties for example:
deprecated- BooleandeprecatedDescription- String
I can also do the change.
WDYT?
There was a problem hiding this comment.
Chiming in that I like the approach with two properties - one for a boolean and the other with a description
There was a problem hiding this comment.
I don't mind either way, it's clear to me if you say deprecated: "reason" that the feature is deprecated (so no need to split).
But I didn't spot anything in the PR that was deprecating something with a string, and therefore these changes seem unrelated to the PR. Also, if unrelated, why are we not getting compilation errors? Maybe because of javascript?
There was a problem hiding this comment.
This is where you can find it: https://github.com/Unleash/unleash/pull/4765/files#diff-b224eb9a2333908c270924576587147061c787791521514cffe3f275b3308871R26
There was a problem hiding this comment.
Ok, and because this is the first time we deprecate an addon, this is still greenfield and it's not something we expose. https://docs.getunleash.io/reference/api/unleash/update-addon
Fixes what this breaks: #4765 - The Datadog integration needs a `flagResolver`.
https://linear.app/unleash/issue/2-1405/remove-slackappaddon-feature-flag-and-make-this-ga
GA's the new Slack App integration by removing the feature flag 馃殌