From 037d8c071cbe759f59e0b0ef7e43c106927f63c2 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Thu, 7 Nov 2019 13:31:52 +0000 Subject: [PATCH 1/5] Document feature flag process This records the feature flag process we intend to use with Riot and also how that interacts with other teams and configuration. Fixes https://github.com/vector-im/riot-web/issues/11116 --- docs/config.md | 7 ++-- docs/feature-flags.md | 89 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 docs/feature-flags.md diff --git a/docs/config.md b/docs/config.md index e609f26de3..d85deabb79 100644 --- a/docs/config.md +++ b/docs/config.md @@ -22,9 +22,10 @@ For a good example, see https://riot.im/develop/config.json. `default_hs_url` is specified. When multiple sources are specified, it is unclear which should take priority and therefore the application cannot continue. * As of Riot 1.4.0, identity servers are optional. See [Identity servers](#identity-servers) below. -1. `features`: Lookup of optional features that may be `enable`d, `disable`d, or exposed to the user - in the `labs` section of settings. The available optional experimental features vary from - release to release. The available features are described in [labs.md](labs.md). +1. `features`: Lookup of optional features that may be `enable`d, `disable`d, or + exposed to the user in the `labs` section of settings. The available + optional experimental features vary from release to release and are (usually) [documented](labs.md). The feature flag process is + [documented](feature-flags.md) as well. 1. `showLabsSettings`: Shows the "labs" tab of user settings even when no `features` are enabled or present. Useful for getting at settings which may be otherwise hidden. 1. `brand`: String to pass to your homeserver when configuring email notifications, to let the diff --git a/docs/feature-flags.md b/docs/feature-flags.md new file mode 100644 index 0000000000..a4dbbea7d2 --- /dev/null +++ b/docs/feature-flags.md @@ -0,0 +1,89 @@ +# Feature flags + +When developing new features for Riot, we use feature flags to give us more +flexibility and control over when and where those features are enabled. + +For example, flags make the following things possible: + +* Extended testing of a feature via labs on develop +* Enabling features when ready instead of the first moment the code is released +* Testing a feature with a specific set of users (by enabling only on a specific + Riot instance) + +The size of the feature controlled by a feature flag may vary widely: it could +be a large project like reactions or a smaller change to an existing algorithm. +A large project might use several feature flags if it's useful to control the +deployment of different portions independently. + +Everyone involved in a feature (engineering, design, product, reviewers) should +think about its deployment plan up front as best as possible so we can have the +right feature flags in place from the start. + +## Interaction with spec process + +Historically, we have often used feature flags to guard client features that +depend on unstable spec features. Unfortunately, there was never clear agreement +about how long such a flag should live for, when it should be removed, etc. + +Under the [new spec +process](https://github.com/matrix-org/matrix-doc/pull/2324), server-side +unstable features can be used by clients and enabled by default as long as +clients commit to doing the associated clean up work once a feature stabilises. + +## Starting work on a feature + +When starting work on a feature, we should create a matching feature flag: + +* Add a new + [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) + of the form: +```js + "feature_cats": { + isFeature: true, + displayName: _td("Adds cats everywhere"), + supportedLevels: LEVELS_FEATURE, + default: false, + }, +``` +* Check whether the feature is enabled as appropriate: +```js + SettingsStore.isFeatureEnabled("feature_cats") +``` +* Add the feature to the [set of labs on develop](../riot.im/develop/config.json): +```json + "features": { + "feature_cats": "labs" + }, +``` +* Document the feature in the [labs documentation](labs.md) + +With these steps completed, the feature is disabled by default, but can be +enabled on develop by interested users for testing. + +Different features may have different deployment plans for when to enable where. The +following lists a few common options. + +## Enabling by default on develop + +Set the feature to `enable` in the [develop config](../riot.im/develop/config.json): + +```json + "features": { + "feature_cats": "enable" + }, +``` + +## Enabling by default on staging and app + +Set the feature to `enable` in the [app config](../riot.im/app/config.json). + +## Feature deployed successfully + +Once we're confident that a feature is working well, we should remove the flag: + +* Remove the [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) +* Remove all `isFeatureEnabled` lines that test for the feature's setting +* Remove the feature from the [labs documentation](labs.md) +* Remove feature state from [develop](../riot.im/develop/config.json) and + [app](../riot.im/app/config.json) configs +* Celebrate! 🥳 From 9ca438087e3c9c3e541e7e30af4525804989632d Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 8 Nov 2019 15:19:23 +0000 Subject: [PATCH 2/5] Add numbers --- docs/feature-flags.md | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index a4dbbea7d2..d49df3c805 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -34,9 +34,9 @@ clients commit to doing the associated clean up work once a feature stabilises. When starting work on a feature, we should create a matching feature flag: -* Add a new - [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) - of the form: +1. Add a new + [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) + of the form: ```js "feature_cats": { isFeature: true, @@ -45,17 +45,17 @@ When starting work on a feature, we should create a matching feature flag: default: false, }, ``` -* Check whether the feature is enabled as appropriate: +2. Check whether the feature is enabled as appropriate: ```js SettingsStore.isFeatureEnabled("feature_cats") ``` -* Add the feature to the [set of labs on develop](../riot.im/develop/config.json): +3. Add the feature to the [set of labs on develop](../riot.im/develop/config.json): ```json "features": { "feature_cats": "labs" }, ``` -* Document the feature in the [labs documentation](labs.md) +4. Document the feature in the [labs documentation](labs.md) With these steps completed, the feature is disabled by default, but can be enabled on develop by interested users for testing. @@ -81,9 +81,9 @@ Set the feature to `enable` in the [app config](../riot.im/app/config.json). Once we're confident that a feature is working well, we should remove the flag: -* Remove the [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) -* Remove all `isFeatureEnabled` lines that test for the feature's setting -* Remove the feature from the [labs documentation](labs.md) -* Remove feature state from [develop](../riot.im/develop/config.json) and +1. Remove the [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) +2. Remove all `isFeatureEnabled` lines that test for the feature's setting +3. Remove the feature from the [labs documentation](labs.md) +4. Remove feature state from [develop](../riot.im/develop/config.json) and [app](../riot.im/app/config.json) configs -* Celebrate! 🥳 +5. Celebrate! 🥳 From fa3d3aa4ddf2d106633e3f31175e3001d84701c5 Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 8 Nov 2019 15:27:02 +0000 Subject: [PATCH 3/5] Absolute URLs --- docs/feature-flags.md | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index d49df3c805..26b9333066 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -49,13 +49,13 @@ When starting work on a feature, we should create a matching feature flag: ```js SettingsStore.isFeatureEnabled("feature_cats") ``` -3. Add the feature to the [set of labs on develop](../riot.im/develop/config.json): +3. Add the feature to the [set of labs on develop](https://github.com/vector-im/riot-web/blob/develop/riot.im/develop/config.json): ```json "features": { "feature_cats": "labs" }, ``` -4. Document the feature in the [labs documentation](labs.md) +4. Document the feature in the [labs documentation](https://github.com/vector-im/riot-web/blob/develop/docs/labs.md) With these steps completed, the feature is disabled by default, but can be enabled on develop by interested users for testing. @@ -65,7 +65,7 @@ following lists a few common options. ## Enabling by default on develop -Set the feature to `enable` in the [develop config](../riot.im/develop/config.json): +Set the feature to `enable` in the [develop config](https://github.com/vector-im/riot-web/blob/develop/riot.im/develop/config.json): ```json "features": { @@ -75,7 +75,7 @@ Set the feature to `enable` in the [develop config](../riot.im/develop/config.js ## Enabling by default on staging and app -Set the feature to `enable` in the [app config](../riot.im/app/config.json). +Set the feature to `enable` in the [app config](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json). ## Feature deployed successfully @@ -83,7 +83,9 @@ Once we're confident that a feature is working well, we should remove the flag: 1. Remove the [setting](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) 2. Remove all `isFeatureEnabled` lines that test for the feature's setting -3. Remove the feature from the [labs documentation](labs.md) -4. Remove feature state from [develop](../riot.im/develop/config.json) and - [app](../riot.im/app/config.json) configs +3. Remove the feature from the [labs documentation](https://github.com/vector-im/riot-web/blob/develop/docs/labs.md) +4. Remove feature state from + [develop](https://github.com/vector-im/riot-web/blob/develop/riot.im/develop/config.json) + and [app](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json) + configs 5. Celebrate! 🥳 From 8b0257d24b950d0daf6a14ab79fd5eb650cbcc3f Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 8 Nov 2019 13:56:31 +0200 Subject: [PATCH 4/5] Apply suggestions from code review Co-Authored-By: Travis Ralston --- docs/config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/config.md b/docs/config.md index d85deabb79..7fa7774e5d 100644 --- a/docs/config.md +++ b/docs/config.md @@ -24,7 +24,7 @@ For a good example, see https://riot.im/develop/config.json. * As of Riot 1.4.0, identity servers are optional. See [Identity servers](#identity-servers) below. 1. `features`: Lookup of optional features that may be `enable`d, `disable`d, or exposed to the user in the `labs` section of settings. The available - optional experimental features vary from release to release and are (usually) [documented](labs.md). The feature flag process is + optional experimental features vary from release to release and are [documented](labs.md). The feature flag process is [documented](feature-flags.md) as well. 1. `showLabsSettings`: Shows the "labs" tab of user settings even when no `features` are enabled or present. Useful for getting at settings which may be otherwise hidden. From d373e2891ecbbbaddcad2e5b55666ad69bd2a3ad Mon Sep 17 00:00:00 2001 From: "J. Ryan Stinnett" Date: Fri, 8 Nov 2019 15:57:46 +0000 Subject: [PATCH 5/5] Describe regular setting path --- docs/feature-flags.md | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/docs/feature-flags.md b/docs/feature-flags.md index 26b9333066..8d31afec10 100644 --- a/docs/feature-flags.md +++ b/docs/feature-flags.md @@ -60,8 +60,8 @@ When starting work on a feature, we should create a matching feature flag: With these steps completed, the feature is disabled by default, but can be enabled on develop by interested users for testing. -Different features may have different deployment plans for when to enable where. The -following lists a few common options. +Different features may have different deployment plans for when to enable where. +The following lists a few common options. ## Enabling by default on develop @@ -75,7 +75,8 @@ Set the feature to `enable` in the [develop config](https://github.com/vector-im ## Enabling by default on staging and app -Set the feature to `enable` in the [app config](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json). +Set the feature to `enable` in the [app +config](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json). ## Feature deployed successfully @@ -89,3 +90,23 @@ Once we're confident that a feature is working well, we should remove the flag: and [app](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json) configs 5. Celebrate! 🥳 + +## Convert to a regular setting (optional) + +Sometimes we decide a feature should always be user-controllable as a setting +even after it has been fully deployed. In that case, we would craft a new, +regular setting: + +1. Remove the feature flag from + [settings](https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/settings/Settings.js) + and add a regular setting with the appropriate levels for your feature +2. Replace the `isFeatureEnabled` lines with `getValue` or similar calls + according to the [settings + docs](https://github.com/matrix-org/matrix-react-sdk/blob/develop/docs/settings.md) + (checking carefully, as we may want a different mix of code paths when the + feature is always present but gated by a setting) +3. Remove the feature from the [labs documentation](https://github.com/vector-im/riot-web/blob/develop/docs/labs.md) +4. Remove feature state from + [develop](https://github.com/vector-im/riot-web/blob/develop/riot.im/develop/config.json) + and [app](https://github.com/vector-im/riot-web/blob/develop/riot.im/app/config.json) + configs