From eb2b895a2091a75b8a3cce35640e7fdbc52afc84 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Sat, 20 May 2017 22:49:55 +0200 Subject: [PATCH] Let's start labeling our CHANGELOG entries Added the type attribute to a CHANGELOG entry. When you create a new entry the software asks for the category of the change and sets the associated type in the file. --- bin/changelog | 136 +++++++++++++----- .../21949-add-type-to-changelog.yml | 4 + doc/development/changelog.md | 54 +++++-- spec/bin/changelog_spec.rb | 112 ++++++++++----- 4 files changed, 223 insertions(+), 83 deletions(-) create mode 100644 changelogs/unreleased/21949-add-type-to-changelog.yml diff --git a/bin/changelog b/bin/changelog index 4c894f8ff5b..61d4de06e90 100755 --- a/bin/changelog +++ b/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) - options = Options.new + Type = Struct.new(:name, :description) + 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| - opts.banner = "Usage: #{__FILE__} [options] [title]\n\n" + class << self + def parse(argv) + options = Options.new - # 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 + parser = OptionParser.new do |opts| + opts.banner = "Usage: #{__FILE__} [options] [title]\n\n" + + # 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| - options.force = value - end + parser.parse!(argv) - opts.on('-m', '--merge-request [integer]', Integer, 'Merge Request ID') do |value| - options.merge_request = value - end + # Title is everything that remains, but let's clean it up a bit + options.title = argv.join(' ').strip.squeeze(' ').tr("\r\n", '') - opts.on('-n', '--dry-run', "Don't actually write anything, just print") do |value| - options.dry_run = value - end + options + end - opts.on('-u', '--git-username', 'Use Git user.name configuration as the author') do |value| - options.author = git_user_name if value - end + def read_type + read_type_message - opts.on('-h', '--help', 'Print help message') do - $stdout.puts opts - exit + type = TYPES[$stdin.getc.to_i - TYPES_OFFSET] + assert_valid_type!(type) + + 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) - - # Title is everything that remains, but let's clean it up a bit - options.title = argv.join(' ').strip.squeeze(' ').tr("\r\n", '') - - options - end - - def self.git_user_name - %x{git config user.name}.strip + def git_user_name + %x{git config user.name}.strip + end 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 diff --git a/changelogs/unreleased/21949-add-type-to-changelog.yml b/changelogs/unreleased/21949-add-type-to-changelog.yml new file mode 100644 index 00000000000..a20f6b7ad4e --- /dev/null +++ b/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 diff --git a/doc/development/changelog.md b/doc/development/changelog.md index ce39a379a0e..f869938fe11 100644 --- a/doc/development/changelog.md +++ b/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 | -| ----------------- | --------- | --------------------------------------------- | -| [`--amend`] | | Amend the previous commit | -| [`--force`] | `-f` | Overwrite an existing entry | -| [`--merge-request`] | `-m` | Set merge request ID | -| [`--dry-run`] | `-n` | Don't actually write anything, just print | -| [`--git-username`] | `-u` | Use Git user.name configuration as the author | -| [`--help`] | `-h` | Print help message | +| Argument | Shorthand | Purpose | +| ----------------- | --------- | ---------------------------------------------------------------------------------------------------------- | +| [`--amend`] | | Amend the previous commit | +| [`--force`] | `-f` | Overwrite an existing entry | +| [`--merge-request`] | `-m` | Set merge request ID | +| [`--dry-run`] | `-n` | Don't actually write anything, just print | +| [`--git-username`] | `-u` | Use Git user.name configuration as the author | +| [`--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 diff --git a/spec/bin/changelog_spec.rb b/spec/bin/changelog_spec.rb index 91aff0db7cc..11cbe70ad29 100644 --- a/spec/bin/changelog_spec.rb +++ b/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 - options = described_class.parse(%w[foo bar --amend]) + describe '.parse' do + it 'parses --amend' do + options = described_class.parse(%w[foo bar --amend]) - expect(options.amend).to eq true - end + expect(options.amend).to eq true + end - it 'parses --force and -f' do - %w[--force -f].each do |flag| - options = described_class.parse(%W[foo #{flag} bar]) + it 'parses --force and -f' do + %w[--force -f].each do |flag| + 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 - %w[--merge-request -m].each do |flag| - options = described_class.parse(%W[foo #{flag} 1234 bar]) + describe '.read_type' do + let(:type) { '1' } - 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 - %w[--dry-run -n].each do |flag| - options = described_class.parse(%W[foo #{flag} bar]) + context 'invalid type given' do + let(:type) { '99' } - 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