Merge branch '21949-add-type-to-changelog' into 'master'

Let's start labeling our CHANGELOG entries

Closes #21949

See merge request !11579

bin/changelog

@@ -14,54 +14,107 @@ Options = Struct.new(
   :dry_run,
   :force,
   :merge_request,
-  :title
+  :title,
+  :type
 )
+INVALID_TYPE = -1
 
 class ChangelogOptionParser
-  def self.parse(argv)
+  Type = Struct.new(:name, :description)
-    options = Options.new
+  TYPES = [
+    Type.new('added', 'New feature'),
+    Type.new('fixed', 'Bug fix'),
+    Type.new('changed', 'Feature change'),
+    Type.new('deprecated', 'New deprecation'),
+    Type.new('removed', 'Feature removal'),
+    Type.new('security', 'Security fix'),
+    Type.new('other', 'Other')
+  ].freeze
+  TYPES_OFFSET = 1
 
-    parser = OptionParser.new do |opts|
+  class << self
-      opts.banner = "Usage: #{__FILE__} [options] [title]\n\n"
+    def parse(argv)
+      options = Options.new
 
-      # Note: We do not provide a shorthand for this in order to match the `git
+      parser = OptionParser.new do |opts|
-      # commit` interface
+        opts.banner = "Usage: #{__FILE__} [options] [title]\n\n"
-      opts.on('--amend', 'Amend the previous commit') do |value|
+
-        options.amend = value
+        # Note: We do not provide a shorthand for this in order to match the `git
+        # commit` interface
+        opts.on('--amend', 'Amend the previous commit') do |value|
+          options.amend = value
+        end
+
+        opts.on('-f', '--force', 'Overwrite an existing entry') do |value|
+          options.force = value
+        end
+
+        opts.on('-m', '--merge-request [integer]', Integer, 'Merge Request ID') do |value|
+          options.merge_request = value
+        end
+
+        opts.on('-n', '--dry-run', "Don't actually write anything, just print") do |value|
+          options.dry_run = value
+        end
+
+        opts.on('-u', '--git-username', 'Use Git user.name configuration as the author') do |value|
+          options.author = git_user_name if value
+        end
+
+        opts.on('-t', '--type [string]', String, "The category of the change, valid options are: #{TYPES.map(&:name).join(', ')}") do |value|
+          options.type = parse_type(value)
+        end
+
+        opts.on('-h', '--help', 'Print help message') do
+          $stdout.puts opts
+          exit
+        end
       end
 
-      opts.on('-f', '--force', 'Overwrite an existing entry') do |value|
+      parser.parse!(argv)
-        options.force = value
-      end
 
-      opts.on('-m', '--merge-request [integer]', Integer, 'Merge Request ID') do |value|
+      # Title is everything that remains, but let's clean it up a bit
-        options.merge_request = value
+      options.title = argv.join(' ').strip.squeeze(' ').tr("\r\n", '')
-      end
 
-      opts.on('-n', '--dry-run', "Don't actually write anything, just print") do |value|
+      options
-        options.dry_run = value
+    end
-      end
 
-      opts.on('-u', '--git-username', 'Use Git user.name configuration as the author') do |value|
+    def read_type
-        options.author = git_user_name if value
+      read_type_message
-      end
 
-      opts.on('-h', '--help', 'Print help message') do
+      type = TYPES[$stdin.getc.to_i - TYPES_OFFSET]
-        $stdout.puts opts
+      assert_valid_type!(type)
-        exit
+
+      type.name
+    end
+
+    private
+
+    def parse_type(name)
+      type_found = TYPES.find do |type|
+        type.name == name
+      end
+      type_found ? type_found.name : INVALID_TYPE
+    end
+
+    def read_type_message
+      $stdout.puts "\n>> Please specify the index for the category of your change:"
+      TYPES.each_with_index do |type, index|
+        $stdout.puts "#{index + TYPES_OFFSET}. #{type.description}"
+      end
+      $stdout.print "\n?> "
+    end
+
+    def assert_valid_type!(type)
+      unless type
+        $stderr.puts "Invalid category index, please select an index between 1 and #{TYPES.length}"
+        exit 1
       end
     end
 
-    parser.parse!(argv)
+    def git_user_name
-
+      %x{git config user.name}.strip
-    # Title is everything that remains, but let's clean it up a bit
+    end
-    options.title = argv.join(' ').strip.squeeze(' ').tr("\r\n", '')
-
-    options
-  end
-
-  def self.git_user_name
-    %x{git config user.name}.strip
   end
 end
 
@@ -72,8 +125,12 @@ class ChangelogEntry
     @options = options
 
     assert_feature_branch!
-    assert_new_file!
     assert_title!
+    assert_new_file!
+
+    # Read type from $stdin unless is already set
+    options.type ||= ChangelogOptionParser.read_type
+    assert_valid_type!
 
     $stdout.puts "\e[32mcreate\e[0m #{file_path}"
     $stdout.puts contents
@@ -90,7 +147,8 @@ class ChangelogEntry
     yaml_content = YAML.dump(
       'title'         => title,
       'merge_request' => options.merge_request,
-      'author'        => options.author
+      'author'        => options.author,
+      'type'          => options.type
     )
     remove_trailing_whitespace(yaml_content)
   end
@@ -129,6 +187,12 @@ class ChangelogEntry
       " to use the title from the previous commit."
   end
 
+  def assert_valid_type!
+    return unless options.type && options.type == INVALID_TYPE
+
+    fail_with 'Invalid category given!'
+  end
+
   def title
     if options.title.empty?
       last_commit_subject

changelogs/unreleased/21949-add-type-to-changelog.yml

@@ -0,0 +1,4 @@
+---
+title: Added type to CHANGELOG entries
+merge_request:
+author: Jacopo Beschi @jacopo-beschi

doc/development/changelog.md

@@ -15,11 +15,14 @@ following format:
 title: "Going through change[log]s"
 merge_request: 1972
 author: Ozzy Osbourne
+type: added
 ```
 
 The `merge_request` value is a reference to a merge request that adds this
 entry, and the `author` key is used to give attribution to community
 contributors. **Both are optional**.
+The `type` field maps the category of the change,
+valid options are: added, fixed, changed, deprecated, removed, security, other. **Type field is mandatory**.
 
 Community contributors and core team members are encouraged to add their name to
 the `author` field. GitLab team members **should not**.
@@ -94,6 +97,19 @@ Its simplest usage is to provide the value for `title`:
 $ bin/changelog 'Hey DZ, I added a feature to GitLab!'
 ```
 
+At this point the script would ask you to select the category of the change (mapped to the `type` field in the entry):
+
+```text
+>> Please specify the category of your change:
+1. New feature
+2. Bug fix
+3. Feature change
+4. New deprecation
+5. Feature removal
+6. Security fix
+7. Other
+```
+
 The entry filename is based on the name of the current Git branch. If you run
 the command above on a branch called `feature/hey-dz`, it will generate a
 `changelogs/unreleased/feature-hey-dz.yml` file.
@@ -106,26 +122,29 @@ create changelogs/unreleased/my-feature.yml
 title: Hey DZ, I added a feature to GitLab!
 merge_request:
 author:
+type:
 ```
 If you're working on the GitLab EE repository, the entry will be added to
 `changelogs/unreleased-ee/` instead.
 
 #### Arguments
 
-| Argument            | Shorthand | Purpose                                       |
+| Argument            | Shorthand | Purpose                                                                                                    |
-| -----------------   | --------- | --------------------------------------------- |
+| -----------------   | --------- | ---------------------------------------------------------------------------------------------------------- |
-| [`--amend`]         |           | Amend the previous commit                     |
+| [`--amend`]         |           | Amend the previous commit                                                                                  |
-| [`--force`]         | `-f`      | Overwrite an existing entry                   |
+| [`--force`]         | `-f`      | Overwrite an existing entry                                                                                |
-| [`--merge-request`] | `-m`      | Set merge request ID                          |
+| [`--merge-request`] | `-m`      | Set merge request ID                                                                                       |
-| [`--dry-run`]       | `-n`      | Don't actually write anything, just print     |
+| [`--dry-run`]       | `-n`      | Don't actually write anything, just print                                                                  |
-| [`--git-username`]  | `-u`      | Use Git user.name configuration as the author |
+| [`--git-username`]  | `-u`      | Use Git user.name configuration as the author                                                              |
-| [`--help`]          | `-h`      | Print help message                            |
+| [`--type`]          | `-t`      | The category of the change, valid options are: added, fixed, changed, deprecated, removed, security, other |
+| [`--help`]          | `-h`      | Print help message                                                                                         |
 
 [`--amend`]: #-amend
 [`--force`]: #-force-or-f
 [`--merge-request`]: #-merge-request-or-m
 [`--dry-run`]: #-dry-run-or-n
 [`--git-username`]: #-git-username-or-u
+[`--type`]: #-type-or-t
 [`--help`]: #-help
 
 ##### `--amend`
@@ -147,6 +166,7 @@ create changelogs/unreleased/feature-hey-dz.yml
 title: Added an awesome new feature to GitLab
 merge_request:
 author:
+type:
 ```
 
 ##### `--force` or `-f`
@@ -164,6 +184,7 @@ create changelogs/unreleased/feature-hey-dz.yml
 title: Hey DZ, I added a feature to GitLab!
 merge_request: 1983
 author:
+type:
 ```
 
 ##### `--merge-request` or `-m`
@@ -178,6 +199,7 @@ create changelogs/unreleased/feature-hey-dz.yml
 title: Hey DZ, I added a feature to GitLab!
 merge_request: 1983
 author:
+type:
 ```
 
 ##### `--dry-run` or `-n`
@@ -192,6 +214,7 @@ create changelogs/unreleased/feature-hey-dz.yml
 title: Added an awesome new feature to GitLab
 merge_request:
 author:
+type:
 
 $ ls changelogs/unreleased/
 ```
@@ -211,6 +234,21 @@ create changelogs/unreleased/feature-hey-dz.yml
 title: Hey DZ, I added a feature to GitLab!
 merge_request:
 author: Jane Doe
+type:
+```
+
+##### `--type` or `-t`
+
+Use the **`--type`** or **`-t`** argument to provide the `type` value:
+
+```text
+$ bin/changelog 'Hey DZ, I added a feature to GitLab!' -t added
+create changelogs/unreleased/feature-hey-dz.yml
+---
+title: Hey DZ, I added a feature to GitLab!
+merge_request:
+author:
+type: added
 ```
 
 ### History and Reasoning

spec/bin/changelog_spec.rb

@@ -4,56 +4,90 @@ load File.expand_path('../../bin/changelog', __dir__)
 
 describe 'bin/changelog' do
   describe ChangelogOptionParser do
-    it 'parses --ammend' do
+    describe '.parse' do
-      options = described_class.parse(%w[foo bar --amend])
+      it 'parses --amend' do
+        options = described_class.parse(%w[foo bar --amend])
 
-      expect(options.amend).to eq true
+        expect(options.amend).to eq true
-    end
+      end
 
-    it 'parses --force and -f' do
+      it 'parses --force and -f' do
-      %w[--force -f].each do |flag|
+        %w[--force -f].each do |flag|
-        options = described_class.parse(%W[foo #{flag} bar])
+          options = described_class.parse(%W[foo #{flag} bar])
 
-        expect(options.force).to eq true
+          expect(options.force).to eq true
+        end
+      end
+
+      it 'parses --merge-request and -m' do
+        %w[--merge-request -m].each do |flag|
+          options = described_class.parse(%W[foo #{flag} 1234 bar])
+
+          expect(options.merge_request).to eq 1234
+        end
+      end
+
+      it 'parses --dry-run and -n' do
+        %w[--dry-run -n].each do |flag|
+          options = described_class.parse(%W[foo #{flag} bar])
+
+          expect(options.dry_run).to eq true
+        end
+      end
+
+      it 'parses --git-username and -u' do
+        allow(described_class).to receive(:git_user_name).and_return('Jane Doe')
+
+        %w[--git-username -u].each do |flag|
+          options = described_class.parse(%W[foo #{flag} bar])
+
+          expect(options.author).to eq 'Jane Doe'
+        end
+      end
+
+      it 'parses --type and -t' do
+        %w[--type -t].each do |flag|
+          options = described_class.parse(%W[foo #{flag} security])
+
+          expect(options.type).to eq 'security'
+        end
+      end
+
+      it 'parses -h' do
+        expect do
+          expect { described_class.parse(%w[foo -h bar]) }.to output.to_stdout
+        end.to raise_error(SystemExit)
+      end
+
+      it 'assigns title' do
+        options = described_class.parse(%W[foo -m 1 bar\n -u baz\r\n --amend])
+
+        expect(options.title).to eq 'foo bar baz'
       end
     end
 
-    it 'parses --merge-request and -m' do
+    describe '.read_type'  do
-      %w[--merge-request -m].each do |flag|
+      let(:type) { '1' }
-        options = described_class.parse(%W[foo #{flag} 1234 bar])
 
-        expect(options.merge_request).to eq 1234
+      it 'reads type from $stdin' do
+        expect($stdin).to receive(:getc).and_return(type)
+        expect do
+          expect(described_class.read_type).to eq('added')
+        end.to output.to_stdout
       end
-    end
 
-    it 'parses --dry-run and -n' do
+      context 'invalid type given' do
-      %w[--dry-run -n].each do |flag|
+        let(:type) { '99' }
-        options = described_class.parse(%W[foo #{flag} bar])
 
-        expect(options.dry_run).to eq true
+        it 'shows error message and exits the program' do
+          allow($stdin).to receive(:getc).and_return(type)
+          expect do
+            expect do
+              expect{ described_class.read_type }.to raise_error(SystemExit)
+            end.to output("Invalid category index, please select an index between 1 and 7\n").to_stderr
+          end.to output.to_stdout
+        end
       end
     end
-
-    it 'parses --git-username and -u' do
-      allow(described_class).to receive(:git_user_name).and_return('Jane Doe')
-
-      %w[--git-username -u].each do |flag|
-        options = described_class.parse(%W[foo #{flag} bar])
-
-        expect(options.author).to eq 'Jane Doe'
-      end
-    end
-
-    it 'parses -h' do
-      expect do
-        expect { described_class.parse(%w[foo -h bar]) }.to output.to_stdout
-      end.to raise_error(SystemExit)
-    end
-
-    it 'assigns title' do
-      options = described_class.parse(%W[foo -m 1 bar\n -u baz\r\n --amend])
-
-      expect(options.title).to eq 'foo bar baz'
-    end
   end
 end