Update feature flag details

243fb8ee · Marcel Amirault · Mike Jang · 7aecd3af · 243fb8ee · 243fb8ee
Commit 243fb8ee authored Sep 16, 2020 by Marcel Amirault Committed by Mike Jang Sep 16, 2020
Showing with 33 additions and 33 deletions

bin/feature-flag bin/feature-flag +8 -8

doc/development/feature_flags/development.md doc/development/feature_flags/development.md +15 -15

spec/bin/feature_flag_spec.rb spec/bin/feature_flag_spec.rb +10 -10

No files found.
--- a/bin/feature-flag
+++ b/bin/feature-flag
@@ -60,11 +60,11 @@ class FeatureFlagOptionParser
          options.force = value
        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
        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
        end

@@ -106,7 +106,7 @@ class FeatureFlagOptionParser

    def read_group
      $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
        $stdout.print "?> "
@@ -114,7 +114,7 @@ class FeatureFlagOptionParser
        group = nil if group.empty?
        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

@@ -123,7 +123,7 @@ class FeatureFlagOptionParser
      return TYPES.first.first if TYPES.one?

      $stdout.puts
-      $stdout.puts ">> Please specify the type of your feature flag:"
+      $stdout.puts ">> Specify the feature flag type:"
      $stdout.puts
      TYPES.each do |type, data|
        $stdout.puts "#{type.to_s.rjust(15)}#{' '*6}#{data[:description]}"
@@ -141,7 +141,7 @@ class FeatureFlagOptionParser

    def read_introduced_by_url
      $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
        $stdout.print "?> "
@@ -166,11 +166,11 @@ class FeatureFlagOptionParser
      issue_new_url = url + "?" + URI.encode_www_form(params)

      $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

-      $stdout.puts ">> Paste URL of `rollout issue` here, or enter to skip:"
+      $stdout.puts ">> URL of the rollout issue (enter to skip):"

      loop do
        $stdout.print "?> "

--- a/doc/development/feature_flags/development.md
+++ b/doc/development/feature_flags/development.md
@@ -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.                                      |
 | `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. |
 | `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. |
@@ -129,16 +129,16 @@ Only feature flags that have a YAML definition file can be used when running the

 ```shell
 $ 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

->> 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

->> 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

->> 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
 create config/feature_flags/development/test-flag.yml
 ---
@@ -305,7 +305,7 @@ used as an actor for `Feature.enabled?`.
 ### Feature flags for licensed features

 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
 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.

 **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,
-the feature flag check will default to `true`.**
+the feature flag check defaults to `true`.**

 NOTE: **Note:**
 Due to limitations with `feature_available?`, the YAML definition for `licensed` feature
@@ -361,9 +361,9 @@ default_enabled: [false, true]

 Feature groups must be defined statically in `lib/feature.rb` (in the
 `.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)

 ### 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
 Feature.enable(:feature_flag_name)
 ```

-Similarly, the following command will disable a feature flag:
+Similarly, the following command disables a feature flag:

 ```ruby
 Feature.disable(:feature_flag_name)
@@ -388,7 +388,7 @@ Feature.enable(:feature_flag_name, Project.find_by_full_path("root/my-project"))

 ## 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**
 to ensure the feature works properly.

@@ -423,10 +423,10 @@ Feature.enabled?(:ci_live_trace, project2) # => false

 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,
-   falling back to default state
-1. There's no way to model that you explicitly disable a specified actor
+   falling back to the default state.
+1. There's no way to model that you explicitly disabled a specified actor.

 ```ruby
 Feature.enable(:my_feature)
@@ -467,7 +467,7 @@ Feature.enable_percentage_of_time(:my_feature_3, 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:

 ```ruby

--- a/spec/bin/feature_flag_spec.rb
+++ b/spec/bin/feature_flag_spec.rb
@@ -135,7 +135,7 @@ RSpec.describe 'bin/feature-flag' do
          expect($stdin).to receive(:gets).and_return(type)
          expect do
            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

        context 'when invalid type is given' do
@@ -147,7 +147,7 @@ RSpec.describe 'bin/feature-flag' do

            expect do
              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
          end
        end
@@ -161,7 +161,7 @@ RSpec.describe 'bin/feature-flag' do
        expect($stdin).to receive(:gets).and_return(group)
        expect do
          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

      context 'invalid group given' do
@@ -173,8 +173,8 @@ RSpec.describe 'bin/feature-flag' do

          expect do
            expect { described_class.read_group }.to raise_error(/EOF/)
-          end.to output(/specify the group/).to_stdout
-            .and output(/Group needs to include/).to_stderr
+          end.to output(/Specify the group introducing the feature flag/).to_stdout
+            .and output(/The group needs to include/).to_stderr
        end
      end
    end
@@ -186,7 +186,7 @@ RSpec.describe 'bin/feature-flag' do
        expect($stdin).to receive(:gets).and_return(url)
        expect do
          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

      context 'empty URL given' do
@@ -196,7 +196,7 @@ RSpec.describe 'bin/feature-flag' do
          expect($stdin).to receive(:gets).and_return(url)
          expect do
            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

@@ -209,7 +209,7 @@ RSpec.describe 'bin/feature-flag' do

          expect do
            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
        end
      end
@@ -223,7 +223,7 @@ RSpec.describe 'bin/feature-flag' do
        expect($stdin).to receive(:gets).and_return(url)
        expect do
          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

      context 'invalid URL given' do
@@ -235,7 +235,7 @@ RSpec.describe 'bin/feature-flag' do

          expect do
            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
        end
      end