Merge branch 'rs-bin-changelog' into 'master'

Add a bin/changelog script and changelog documentation

See merge request !7098

CONTRIBUTING.md

@@ -246,13 +246,7 @@ request is as follows:
 1. Fork the project into your personal space on GitLab.com
 1. Create a feature branch, branch away from `master`
 1. Write [tests](https://gitlab.com/gitlab-org/gitlab-development-kit#running-the-tests) and code
-1. Add your changes to the [CHANGELOG.md](CHANGELOG.md):
-  1. If you are fixing a ~regression issue, you can add your entry to the next
-     patch release (e.g. `8.12.5` if current version is `8.12.4`)
-  1. Otherwise, add your entry to the next minor release (e.g. `8.13.0` if
-     current version is `8.12.4`
-  1. Please add your entry at a random place among the entries of the targeted
-     release
+1. [Generate a changelog entry with `bin/changelog`][changelog]
 1. If you are writing documentation, make sure to follow the
    [documentation styleguide][doc-styleguide]
 1. If you have multiple commits please combine them into one commit by
@@ -471,6 +465,7 @@ available at [http://contributor-covenant.org/version/1/1/0/](http://contributor
 [contributor-covenant]: http://contributor-covenant.org
 [rss-source]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#source-code-layout
 [rss-naming]: https://github.com/bbatsov/ruby-style-guide/blob/master/README.md#naming
+[changelog]: doc/development/changelog.md "Generate a changelog entry"
 [doc-styleguide]: doc/development/doc_styleguide.md "Documentation styleguide"
 [scss-styleguide]: doc/development/scss_styleguide.md "SCSS styleguide"
 [newlines-styleguide]: doc/development/newlines_styleguide.md "Newlines styleguide"

bin/changelog

+#!/usr/bin/env ruby
+#
+# Generate a changelog entry file in the correct location.
+#
+# Automatically stages the file and amends the previous commit if the `--amend`
+# argument is used.
+
+require 'optparse'
+require 'yaml'
+
+Options = Struct.new(
+  :amend,
+  :author,
+  :dry_run,
+  :merge_request,
+  :title
+)
+
+class ChangelogOptionParser
+  def self.parse(argv)
+    options = Options.new
+
+    parser = OptionParser.new do |opts|
+      opts.banner = "Usage: #{__FILE__} [options]"
+
+      # 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('-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('-h', '--help', 'Print help message') do
+        $stdout.puts opts
+        exit
+      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
+  end
+end
+
+class ChangelogEntry
+  attr_reader :options
+
+  def initialize(options)
+    @options = options
+
+    assert_feature_branch!
+    assert_new_file!
+    assert_title!
+
+    $stdout.puts "\e[32mcreate\e[0m #{file_path}"
+    $stdout.puts contents
+
+    unless options.dry_run
+      write
+      amend_commit if options.amend
+    end
+  end
+
+  def contents
+    YAML.dump(
+      'title'         => title,
+      'merge_request' => options.merge_request,
+      'author'        => options.author
+    )
+  end
+
+  def write
+    File.write(file_path, contents)
+  end
+
+  def amend_commit
+    %x{git add #{file_path}}
+    exec("git commit --amend")
+  end
+
+  private
+
+  def fail_with(message)
+    $stderr.puts "\e[31merror\e[0m #{message}"
+    exit 1
+  end
+
+  def assert_feature_branch!
+    return unless branch_name == 'master'
+
+    fail_with "Create a branch first!"
+  end
+
+  def assert_new_file!
+    return unless File.exist?(file_path)
+
+    fail_with "#{file_path} already exists!"
+  end
+
+  def assert_title!
+    return if options.title.length > 0 || options.amend
+
+    fail_with "Provide a title for the changelog entry or use `--amend`" \
+      " to use the title from the previous commit."
+  end
+
+  def title
+    if options.title.empty?
+      last_commit_subject
+    else
+      options.title
+    end
+  end
+
+  def last_commit_subject
+    %x{git log --format="%s" -1}.strip
+  end
+
+  def file_path
+    File.join(
+      unreleased_path,
+      branch_name.gsub(/[^\w-]/, '-') << '.yml'
+    )
+  end
+
+  def unreleased_path
+    File.join('changelogs', 'unreleased').tap do |path|
+      path << '-ee' if ee?
+    end
+  end
+
+  def ee?
+    @ee ||= File.exist?(File.expand_path('../CHANGELOG-EE.md', __dir__))
+  end
+
+  def branch_name
+    @branch_name ||= %x{git symbolic-ref HEAD}.strip.sub(%r{\Arefs/heads/}, '')
+  end
+end
+
+if $0 == __FILE__
+  options = ChangelogOptionParser.parse(ARGV)
+  ChangelogEntry.new(options)
+end
+
+# vim: ft=ruby

doc/development/README.md

@@ -21,6 +21,7 @@
 
 ## Process
 
+- [Generate a changelog entry with `bin/changelog`](changelog.md)
 - [Code review guidelines](code_review.md) for reviewing code and having code reviewed.
 - [Merge request performance guidelines](merge_request_performance_guidelines.md)
   for ensuring merge requests do not negatively impact GitLab performance

doc/development/changelog.md

+# Generate a changelog entry
+
+This guide contains instructions for generating a changelog entry data file, as
+well as information and history about our changelog process.
+
+## Overview
+
+Each bullet point, or **entry**, in our [`CHANGELOG.md`][changelog.md] file is
+generated from a single data file in the [`changelogs/unreleased/`][unreleased]
+(or corresponding EE) folder. The file is expected to be a [YAML] file in the
+following format:
+
+```yaml
+---
+title: "Going through change[log]s"
+merge_request: 1972
+author: Ozzy Osbourne
+```
+
+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.
+
+If you're working on the GitLab EE repository, the entry will be added to
+`changelogs/unreleased-ee/` instead.
+
+[changelog.md]: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG.md
+[unreleased]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/changelogs/
+[YAML]: https://en.wikipedia.org/wiki/YAML
+
+## Instructions
+
+A `bin/changelog` script is available to generate the changelog entry file
+automatically.
+
+Its simplest usage is to provide the value for `title`:
+
+```text
+$ bin/changelog Hey DZ, I added a feature to GitLab!
+create changelogs/unreleased/my-feature.yml
+---
+title: Hey DZ, I added a feature to GitLab!
+merge_request:
+author:
+```
+
+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` file.
+
+### Arguments
+
+| Argument          | Shorthand | Purpose                                       |
+| ----------------- | --------- | --------------------------------------------- |
+| `--amend`         |           | Amend the previous commit                     |
+| `--merge-request` | `-m`      | 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                            |
+
+#### `--amend`
+
+You can pass the **`--amend`** argument to automatically stage the generated
+file and amend it to the previous commit.
+
+If you use **`--amend`** and don't provide a title, it will automatically use
+the "subject" of the previous commit, which is the first line of the commit
+message:
+
+```text
+$ git show --oneline
+ab88683 Added an awesome new feature to GitLab
+
+$ bin/changelog --amend
+create changelogs/unreleased/feature-hey-dz.yml
+---
+title: Added an awesome new feature to GitLab
+merge_request:
+author:
+```
+
+#### `--merge-request` or `-m`
+
+Use the **`--merge-request`** or **`-m`** argument to provide the
+`merge_request` value:
+
+```text
+$ bin/changelog Hey DZ, I added a feature to GitLab! -m 1983
+create changelogs/unreleased/feature-hey-dz.yml
+---
+title: Hey DZ, I added a feature to GitLab!
+merge_request: 1983
+author:
+```
+
+#### `--dry-run` or `-n`
+
+Use the **`--dry-run`** or **`-n`** argument to prevent actually writing or
+committing anything:
+
+```text
+$ bin/changelog --amend --dry-run
+create changelogs/unreleased/feature-hey-dz.yml
+---
+title: Added an awesome new feature to GitLab
+merge_request:
+author:
+
+$ ls changelogs/unreleased/
+```
+
+#### `--git-username` or `-u`
+
+Use the **`--git-username`** or **`-u`** argument to automatically fill in the
+`author` value with your configured Git `user.name` value:
+
+```text
+$ git config user.name
+Jane Doe
+
+$ bin/changelog --u Hey DZ, I added a feature to GitLab!
+create changelogs/unreleased/feature-hey-dz.yml
+---
+title: Hey DZ, I added a feature to GitLab!
+merge_request:
+author: Jane Doe
+```
+
+## History and Reasoning
+
+Our `CHANGELOG` file was previously updated manually by each contributor that
+felt their change warranted an entry. When two merge requests added their own
+entries at the same spot in the list, it created a merge conflict in one as soon
+as the other was merged. When we had dozens of merge requests fighting for the
+same changelog entry location, this quickly became a major source of merge
+conflicts and delays in development.
+
+This led us to a [boring solution] of "add your entry in a random location in
+the list." This actually worked pretty well as we got further along in each
+monthly release cycle, but at the start of a new cycle, when a new version
+section was added and there were fewer places to "randomly" add an entry, the
+conflicts became a problem again until we had a sufficient number of entries.
+
+On top of all this, it created an entirely different headache for [release managers]
+when they cherry-picked a commit into a stable branch for a patch release. If
+the commit included an entry in the `CHANGELOG`, it would include the entire
+changelog for the latest version in `master`, so the release manager would have
+to manually remove the later entries. They often would have had to do this
+multiple times per patch release. This was compounded when we had to release
+multiple patches at once due to a security issue.
+
+We needed to automate all of this manual work. So we [started brainstorming].
+After much discussion we settled on the current solution of one file per entry,
+and then compiling the entries into the overall `CHANGELOG.md` file during the
+[release process].
+
+[boring solution]: https://about.gitlab.com/handbook/#boring-solutions
+[release managers]: https://gitlab.com/gitlab-org/release-tools/blob/master/doc/release-manager.md
+[started brainstorming]: https://gitlab.com/gitlab-org/gitlab-ce/issues/17826
+[release process]: https://gitlab.com/gitlab-org/release-tools
+
+---
+
+[Return to Development documentation](README.md)

spec/bin/changelog_spec.rb

+require 'spec_helper'
+
+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])
+
+      expect(options.amend).to eq true
+    end
+
+    it 'parses --merge-request' do
+      options = described_class.parse(%w[foo --merge-request 1234 bar])
+
+      expect(options.merge_request).to eq 1234
+    end
+
+    it 'parses -m' do
+      options = described_class.parse(%w[foo -m 4321 bar])
+
+      expect(options.merge_request).to eq 4321
+    end
+
+    it 'parses --dry-run' do
+      options = described_class.parse(%w[foo --dry-run bar])
+
+      expect(options.dry_run).to eq true
+    end
+
+    it 'parses -n' do
+      options = described_class.parse(%w[foo -n bar])
+
+      expect(options.dry_run).to eq true
+    end
+
+    it 'parses --git-username' do
+      allow(described_class).to receive(:git_user_name).and_return('Jane Doe')
+      options = described_class.parse(%w[foo --git-username bar])
+
+      expect(options.author).to eq 'Jane Doe'
+    end
+
+    it 'parses -u' do
+      allow(described_class).to receive(:git_user_name).and_return('John Smith')
+      options = described_class.parse(%w[foo -u bar])
+
+      expect(options.author).to eq 'John Smith'
+    end
+
+    it 'parses -h' do
+      expect do
+        $stdout = StringIO.new
+
+        described_class.parse(%w[foo -h bar])
+      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