Commit 4fbf89e6 authored by Mike Jang's avatar Mike Jang

Merge branch 'update-feature-flag-details' into 'master'

Update feature flag details

Closes #232780

See merge request gitlab-org/gitlab!42322
parents ea4b9d1e 243fb8ee
...@@ -60,11 +60,11 @@ class FeatureFlagOptionParser ...@@ -60,11 +60,11 @@ class FeatureFlagOptionParser
options.force = value options.force = value
end end
opts.on('-m', '--introduced-by-url [string]', String, 'URL to Merge Request introducing Feature Flag') do |value| opts.on('-m', '--introduced-by-url [string]', String, 'URL of Merge Request introducing the Feature Flag') do |value|
options.introduced_by_url = value options.introduced_by_url = value
end end
opts.on('-i', '--rollout-issue-url [string]', String, 'URL to Issue rolling out Feature Flag') do |value| opts.on('-i', '--rollout-issue-url [string]', String, 'URL of Issue rolling out the Feature Flag') do |value|
options.rollout_issue_url = value options.rollout_issue_url = value
end end
...@@ -106,7 +106,7 @@ class FeatureFlagOptionParser ...@@ -106,7 +106,7 @@ class FeatureFlagOptionParser
def read_group def read_group
$stdout.puts $stdout.puts
$stdout.puts ">> Please specify the group introducing feature flag, like `group::apm`:" $stdout.puts ">> Specify the group introducing the feature flag, like `group::apm`:"
loop do loop do
$stdout.print "?> " $stdout.print "?> "
...@@ -114,7 +114,7 @@ class FeatureFlagOptionParser ...@@ -114,7 +114,7 @@ class FeatureFlagOptionParser
group = nil if group.empty? group = nil if group.empty?
return group if group.nil? || group.start_with?('group::') return group if group.nil? || group.start_with?('group::')
$stderr.puts "Group needs to include `group::`" $stderr.puts "The group needs to include `group::`"
end end
end end
...@@ -123,7 +123,7 @@ class FeatureFlagOptionParser ...@@ -123,7 +123,7 @@ class FeatureFlagOptionParser
return TYPES.first.first if TYPES.one? return TYPES.first.first if TYPES.one?
$stdout.puts $stdout.puts
$stdout.puts ">> Please specify the type of your feature flag:" $stdout.puts ">> Specify the feature flag type:"
$stdout.puts $stdout.puts
TYPES.each do |type, data| TYPES.each do |type, data|
$stdout.puts "#{type.to_s.rjust(15)}#{' '*6}#{data[:description]}" $stdout.puts "#{type.to_s.rjust(15)}#{' '*6}#{data[:description]}"
...@@ -141,7 +141,7 @@ class FeatureFlagOptionParser ...@@ -141,7 +141,7 @@ class FeatureFlagOptionParser
def read_introduced_by_url def read_introduced_by_url
$stdout.puts $stdout.puts
$stdout.puts ">> If you have MR open, can you paste the URL here? (or enter to skip)" $stdout.puts ">> URL of the MR introducing the feature flag (enter to skip):"
loop do loop do
$stdout.print "?> " $stdout.print "?> "
...@@ -166,11 +166,11 @@ class FeatureFlagOptionParser ...@@ -166,11 +166,11 @@ class FeatureFlagOptionParser
issue_new_url = url + "?" + URI.encode_www_form(params) issue_new_url = url + "?" + URI.encode_www_form(params)
$stdout.puts $stdout.puts
$stdout.puts ">> Open this URL and fill the rest of details:" $stdout.puts ">> Open this URL and fill in the rest of the details:"
$stdout.puts issue_new_url $stdout.puts issue_new_url
$stdout.puts $stdout.puts
$stdout.puts ">> Paste URL of `rollout issue` here, or enter to skip:" $stdout.puts ">> URL of the rollout issue (enter to skip):"
loop do loop do
$stdout.print "?> " $stdout.print "?> "
......
...@@ -110,7 +110,7 @@ Each feature flag is defined in a separate YAML file consisting of a number of f ...@@ -110,7 +110,7 @@ Each feature flag is defined in a separate YAML file consisting of a number of f
|---------------------|----------|----------------------------------------------------------------| |---------------------|----------|----------------------------------------------------------------|
| `name` | yes | Name of the feature flag. | | `name` | yes | Name of the feature flag. |
| `type` | yes | Type of feature flag. | | `type` | yes | Type of feature flag. |
| `default_enabled` | yes | The default state of the feature flag that is strongly validated, with `default_enabled:` passed as an argument. | | `default_enabled` | yes | The default state of the feature flag that is strictly validated, with `default_enabled:` passed as an argument. |
| `introduced_by_url` | no | The URL to the Merge Request that introduced the feature flag. | | `introduced_by_url` | no | The URL to the Merge Request that introduced the feature flag. |
| `rollout_issue_url` | no | The URL to the Issue covering the feature flag rollout. | | `rollout_issue_url` | no | The URL to the Issue covering the feature flag rollout. |
| `group` | no | The [group](https://about.gitlab.com/handbook/product/product-categories/#devops-stages) that owns the feature flag. | | `group` | no | The [group](https://about.gitlab.com/handbook/product/product-categories/#devops-stages) that owns the feature flag. |
...@@ -129,16 +129,16 @@ Only feature flags that have a YAML definition file can be used when running the ...@@ -129,16 +129,16 @@ Only feature flags that have a YAML definition file can be used when running the
```shell ```shell
$ bin/feature-flag my-feature-flag $ bin/feature-flag my-feature-flag
>> Please specify the group introducing feature flag, like `group::apm`: >> Specify the group introducing the feature flag, like `group::apm`:
?> group::memory ?> group::memory
>> If you have MR open, can you paste the URL here? (or enter to skip) >> URL of the MR introducing the feature flag (enter to skip):
?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602 ?> https://gitlab.com/gitlab-org/gitlab/-/merge_requests/38602
>> Open this URL and fill the rest of details: >> Open this URL and fill in the rest of the details:
https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue%5Btitle%5D=%5BFeature+flag%5D+Rollout+of+%60test-flag%60&issuable_template=Feature+Flag+Roll+Out https://gitlab.com/gitlab-org/gitlab/-/issues/new?issue%5Btitle%5D=%5BFeature+flag%5D+Rollout+of+%60test-flag%60&issuable_template=Feature+Flag+Roll+Out
>> Paste URL of `rollout issue` here, or enter to skip: >> URL of the rollout issue (enter to skip):
?> https://gitlab.com/gitlab-org/gitlab/-/issues/232533 ?> https://gitlab.com/gitlab-org/gitlab/-/issues/232533
create config/feature_flags/development/test-flag.yml create config/feature_flags/development/test-flag.yml
--- ---
...@@ -305,7 +305,7 @@ used as an actor for `Feature.enabled?`. ...@@ -305,7 +305,7 @@ used as an actor for `Feature.enabled?`.
### Feature flags for licensed features ### Feature flags for licensed features
If a feature is license-gated, there's no need to add an additional If a feature is license-gated, there's no need to add an additional
explicit feature flag check since the flag will be checked as part of the explicit feature flag check since the flag is checked as part of the
`License.feature_available?` call. Similarly, there's no need to "clean up" a `License.feature_available?` call. Similarly, there's no need to "clean up" a
feature flag once the feature has reached general availability. feature flag once the feature has reached general availability.
...@@ -316,7 +316,7 @@ a by default enabled feature flag with the same name as the provided argument. ...@@ -316,7 +316,7 @@ a by default enabled feature flag with the same name as the provided argument.
**An important side-effect of the implicit feature flags mentioned above is that **An important side-effect of the implicit feature flags mentioned above is that
unless the feature is explicitly disabled or limited to a percentage of users, unless the feature is explicitly disabled or limited to a percentage of users,
the feature flag check will default to `true`.** the feature flag check defaults to `true`.**
NOTE: **Note:** NOTE: **Note:**
Due to limitations with `feature_available?`, the YAML definition for `licensed` feature Due to limitations with `feature_available?`, the YAML definition for `licensed` feature
...@@ -361,9 +361,9 @@ default_enabled: [false, true] ...@@ -361,9 +361,9 @@ default_enabled: [false, true]
Feature groups must be defined statically in `lib/feature.rb` (in the Feature groups must be defined statically in `lib/feature.rb` (in the
`.register_feature_groups` method), but their implementation can obviously be `.register_feature_groups` method), but their implementation can obviously be
dynamic (querying the DB etc.). dynamic (querying the DB, for example).
Once defined in `lib/feature.rb`, you will be able to activate a Once defined in `lib/feature.rb`, you can to activate a
feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature) feature for a given feature group via the [`feature_group` parameter of the features API](../../api/features.md#set-or-create-a-feature)
### Enabling a feature flag locally (in development) ### Enabling a feature flag locally (in development)
...@@ -374,7 +374,7 @@ In the rails console (`rails c`), enter the following command to enable a featur ...@@ -374,7 +374,7 @@ In the rails console (`rails c`), enter the following command to enable a featur
Feature.enable(:feature_flag_name) Feature.enable(:feature_flag_name)
``` ```
Similarly, the following command will disable a feature flag: Similarly, the following command disables a feature flag:
```ruby ```ruby
Feature.disable(:feature_flag_name) Feature.disable(:feature_flag_name)
...@@ -388,7 +388,7 @@ Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project")) ...@@ -388,7 +388,7 @@ Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))
## Feature flags in tests ## Feature flags in tests
Introducing a feature flag into the codebase creates an additional codepath that should be tested. Introducing a feature flag into the codebase creates an additional code path that should be tested.
It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled** It is strongly advised to test all code affected by a feature flag, both when **enabled** and **disabled**
to ensure the feature works properly. to ensure the feature works properly.
...@@ -423,10 +423,10 @@ Feature.enabled?(:ci_live_trace, project2) # => false ...@@ -423,10 +423,10 @@ Feature.enabled?(:ci_live_trace, project2) # => false
The behavior of FlipperGate is as follows: The behavior of FlipperGate is as follows:
1. You can enable an override for a specified actor to be enabled 1. You can enable an override for a specified actor to be enabled.
1. You can disable (remove) an override for a specified actor, 1. You can disable (remove) an override for a specified actor,
falling back to default state falling back to the default state.
1. There's no way to model that you explicitly disable a specified actor 1. There's no way to model that you explicitly disabled a specified actor.
```ruby ```ruby
Feature.enable(:my_feature) Feature.enable(:my_feature)
...@@ -467,7 +467,7 @@ Feature.enable_percentage_of_time(:my_feature_3, 50) ...@@ -467,7 +467,7 @@ Feature.enable_percentage_of_time(:my_feature_3, 50)
Feature.enable_percentage_of_actors(:my_feature_4, 50) Feature.enable_percentage_of_actors(:my_feature_4, 50)
``` ```
Each feature flag that has a defined state will be persisted Each feature flag that has a defined state is persisted
during test execution time: during test execution time:
```ruby ```ruby
......
...@@ -135,7 +135,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -135,7 +135,7 @@ RSpec.describe 'bin/feature-flag' do
expect($stdin).to receive(:gets).and_return(type) expect($stdin).to receive(:gets).and_return(type)
expect do expect do
expect(described_class.read_type).to eq(:development) expect(described_class.read_type).to eq(:development)
end.to output(/specify the type/).to_stdout end.to output(/Specify the feature flag type/).to_stdout
end end
context 'when invalid type is given' do context 'when invalid type is given' do
...@@ -147,7 +147,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -147,7 +147,7 @@ RSpec.describe 'bin/feature-flag' do
expect do expect do
expect { described_class.read_type }.to raise_error(/EOF/) expect { described_class.read_type }.to raise_error(/EOF/)
end.to output(/specify the type/).to_stdout end.to output(/Specify the feature flag type/).to_stdout
.and output(/Invalid type specified/).to_stderr .and output(/Invalid type specified/).to_stderr
end end
end end
...@@ -161,7 +161,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -161,7 +161,7 @@ RSpec.describe 'bin/feature-flag' do
expect($stdin).to receive(:gets).and_return(group) expect($stdin).to receive(:gets).and_return(group)
expect do expect do
expect(described_class.read_group).to eq('group::memory') expect(described_class.read_group).to eq('group::memory')
end.to output(/specify the group/).to_stdout end.to output(/Specify the group introducing the feature flag/).to_stdout
end end
context 'invalid group given' do context 'invalid group given' do
...@@ -173,8 +173,8 @@ RSpec.describe 'bin/feature-flag' do ...@@ -173,8 +173,8 @@ RSpec.describe 'bin/feature-flag' do
expect do expect do
expect { described_class.read_group }.to raise_error(/EOF/) expect { described_class.read_group }.to raise_error(/EOF/)
end.to output(/specify the group/).to_stdout end.to output(/Specify the group introducing the feature flag/).to_stdout
.and output(/Group needs to include/).to_stderr .and output(/The group needs to include/).to_stderr
end end
end end
end end
...@@ -186,7 +186,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -186,7 +186,7 @@ RSpec.describe 'bin/feature-flag' do
expect($stdin).to receive(:gets).and_return(url) expect($stdin).to receive(:gets).and_return(url)
expect do expect do
expect(described_class.read_introduced_by_url).to eq('https://merge-request') expect(described_class.read_introduced_by_url).to eq('https://merge-request')
end.to output(/can you paste the URL here/).to_stdout end.to output(/URL of the MR introducing the feature flag/).to_stdout
end end
context 'empty URL given' do context 'empty URL given' do
...@@ -196,7 +196,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -196,7 +196,7 @@ RSpec.describe 'bin/feature-flag' do
expect($stdin).to receive(:gets).and_return(url) expect($stdin).to receive(:gets).and_return(url)
expect do expect do
expect(described_class.read_introduced_by_url).to be_nil expect(described_class.read_introduced_by_url).to be_nil
end.to output(/can you paste the URL here/).to_stdout end.to output(/URL of the MR introducing the feature flag/).to_stdout
end end
end end
...@@ -209,7 +209,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -209,7 +209,7 @@ RSpec.describe 'bin/feature-flag' do
expect do expect do
expect { described_class.read_introduced_by_url }.to raise_error(/EOF/) expect { described_class.read_introduced_by_url }.to raise_error(/EOF/)
end.to output(/can you paste the URL here/).to_stdout end.to output(/URL of the MR introducing the feature flag/).to_stdout
.and output(/URL needs to start with/).to_stderr .and output(/URL needs to start with/).to_stderr
end end
end end
...@@ -223,7 +223,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -223,7 +223,7 @@ RSpec.describe 'bin/feature-flag' do
expect($stdin).to receive(:gets).and_return(url) expect($stdin).to receive(:gets).and_return(url)
expect do expect do
expect(described_class.read_rollout_issue_url(options)).to eq('https://issue') expect(described_class.read_rollout_issue_url(options)).to eq('https://issue')
end.to output(/Paste URL of `rollout issue` here/).to_stdout end.to output(/URL of the rollout issue/).to_stdout
end end
context 'invalid URL given' do context 'invalid URL given' do
...@@ -235,7 +235,7 @@ RSpec.describe 'bin/feature-flag' do ...@@ -235,7 +235,7 @@ RSpec.describe 'bin/feature-flag' do
expect do expect do
expect { described_class.read_rollout_issue_url(options) }.to raise_error(/EOF/) expect { described_class.read_rollout_issue_url(options) }.to raise_error(/EOF/)
end.to output(/Paste URL of `rollout issue` here/).to_stdout end.to output(/URL of the rollout issue/).to_stdout
.and output(/URL needs to start/).to_stderr .and output(/URL needs to start/).to_stderr
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment