Merge branch 'master' into notebooklab-in-repo

2017-04-13 08:01:55 +01:00 · 2017-04-13 08:01:55 +01:00 · 2c643a552e
parent bf5248ac28 d7a527163b
commit 2c643a552e
162 changed files with 3369 additions and 1245 deletions
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@ -1,4 +1,4 @@
-image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-git-2.7-phantomjs-2.1-node-7.1"
+image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.3.3-golang-1.8-git-2.7-phantomjs-2.1-node-7.1"

 cache:
  key: "ruby-233"
--- a/.gitlab/issue_templates/Bug.md
+++ b/.gitlab/issue_templates/Bug.md
@ -25,14 +25,20 @@ logs, and code as it's very hard to read otherwise.)

 #### Results of GitLab environment info

+<details>
+
 (For installations with omnibus-gitlab package run and paste the output of:
 `sudo gitlab-rake gitlab:env:info`)

 (For installations from source run and paste the output of:
 `sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)

+</details>
+
 #### Results of GitLab application Check

+<details>
+
 (For installations with omnibus-gitlab package run and paste the output of:
 `sudo gitlab-rake gitlab:check SANITIZE=true`)

@ -41,6 +47,8 @@ logs, and code as it's very hard to read otherwise.)

 (we will only investigate if the tests are passing)

+</details>
+
 ### Possible fixes

 (If you can, link to the line of code that might be responsible for the problem)
--- a/Gemfile.lock
+++ b/Gemfile.lock
@ -330,7 +330,7 @@ GEM
    grape-entity (0.6.0)
      activesupport
      multi_json (>= 1.3.2)
-    grpc (1.1.2)
+    grpc (1.2.2)
      google-protobuf (~> 3.1)
      googleauth (~> 0.5.1)
    haml (4.0.7)
--- a/app/assets/javascripts/gfm_auto_complete.js
+++ b/app/assets/javascripts/gfm_auto_complete.js
@ -50,7 +50,7 @@ window.gl.GfmAutoComplete = {
    template: '<li>${title}</li>'
  },
  Loading: {
-    template: '<li style="pointer-events: none;"><i class="fa fa-refresh fa-spin"></i> Loading...</li>'
+    template: '<li style="pointer-events: none;"><i class="fa fa-spinner fa-spin"></i> Loading...</li>'
  },
  DefaultOptions: {
    sorter: function(query, items, searchKey) {
--- a/app/assets/javascripts/issue.js
+++ b/app/assets/javascripts/issue.js
@ -20,57 +20,60 @@ class Issue {
      });
      Issue.initIssueBtnEventListeners();
    }
+
+    Issue.$btnNewBranch = $('#new-branch');
+
    Issue.initMergeRequests();
    Issue.initRelatedBranches();
    Issue.initCanCreateBranch();
  }

  static initIssueBtnEventListeners() {
-    var issueFailMessage;
-    issueFailMessage = 'Unable to update this issue at this time.';
-    return $('a.btn-close, a.btn-reopen').on('click', function(e) {
-      var $this, isClose, shouldSubmit, url;
+    const issueFailMessage = 'Unable to update this issue at this time.';
+
+    const closeButtons = $('a.btn-close');
+    const isClosedBadge = $('div.status-box-closed');
+    const isOpenBadge = $('div.status-box-open');
+    const projectIssuesCounter = $('.issue_counter');
+    const reopenButtons = $('a.btn-reopen');
+
+    return closeButtons.add(reopenButtons).on('click', function(e) {
+      var $this, shouldSubmit, url;
      e.preventDefault();
      e.stopImmediatePropagation();
      $this = $(this);
-      isClose = $this.hasClass('btn-close');
      shouldSubmit = $this.hasClass('btn-comment');
      if (shouldSubmit) {
        Issue.submitNoteForm($this.closest('form'));
      }
      $this.prop('disabled', true);
+      Issue.setNewBranchButtonState(true, null);
      url = $this.attr('href');
      return $.ajax({
        type: 'PUT',
-        url: url,
-        error: function(jqXHR, textStatus, errorThrown) {
-          var issueStatus;
-          issueStatus = isClose ? 'close' : 'open';
-          return new Flash(issueFailMessage, 'alert');
-        },
-        success: function(data, textStatus, jqXHR) {
-          if ('id' in data) {
-            $(document).trigger('issuable:change');
-            let total = Number($('.issue_counter').text().replace(/[^\d]/, ''));
-            if (isClose) {
-              $('a.btn-close').addClass('hidden');
-              $('a.btn-reopen').removeClass('hidden');
-              $('div.status-box-closed').removeClass('hidden');
-              $('div.status-box-open').addClass('hidden');
-              total -= 1;
-            } else {
-              $('a.btn-reopen').addClass('hidden');
-              $('a.btn-close').removeClass('hidden');
-              $('div.status-box-closed').addClass('hidden');
-              $('div.status-box-open').removeClass('hidden');
-              total += 1;
-            }
-            $('.issue_counter').text(gl.text.addDelimiter(total));
-          } else {
-            new Flash(issueFailMessage, 'alert');
-          }
-          return $this.prop('disabled', false);
+        url: url
+      }).fail(function(jqXHR, textStatus, errorThrown) {
+        new Flash(issueFailMessage);
+        Issue.initCanCreateBranch();
+      }).done(function(data, textStatus, jqXHR) {
+        if ('id' in data) {
+          $(document).trigger('issuable:change');
+
+          const isClosed = $this.hasClass('btn-close');
+          closeButtons.toggleClass('hidden', isClosed);
+          reopenButtons.toggleClass('hidden', !isClosed);
+          isClosedBadge.toggleClass('hidden', !isClosed);
+          isOpenBadge.toggleClass('hidden', isClosed);
+
+          let numProjectIssues = Number(projectIssuesCounter.text().replace(/[^\d]/, ''));
+          numProjectIssues = isClosed ? numProjectIssues - 1 : numProjectIssues + 1;
+          projectIssuesCounter.text(gl.text.addDelimiter(numProjectIssues));
+        } else {
+          new Flash(issueFailMessage);
        }
+
+        $this.prop('disabled', false);
+        Issue.initCanCreateBranch();
      });
    });
  }
@ -86,9 +89,9 @@ class Issue {
  static initMergeRequests() {
    var $container;
    $container = $('#merge-requests');
-    return $.getJSON($container.data('url')).error(function() {
-      return new Flash('Failed to load referenced merge requests', 'alert');
-    }).success(function(data) {
+    return $.getJSON($container.data('url')).fail(function() {
+      return new Flash('Failed to load referenced merge requests');
+    }).done(function(data) {
      if ('html' in data) {
        return $container.html(data.html);
      }
@ -98,9 +101,9 @@ class Issue {
  static initRelatedBranches() {
    var $container;
    $container = $('#related-branches');
-    return $.getJSON($container.data('url')).error(function() {
-      return new Flash('Failed to load related branches', 'alert');
-    }).success(function(data) {
+    return $.getJSON($container.data('url')).fail(function() {
+      return new Flash('Failed to load related branches');
+    }).done(function(data) {
      if ('html' in data) {
        return $container.html(data.html);
      }
@ -108,24 +111,27 @@ class Issue {
  }

  static initCanCreateBranch() {
-    var $container;
-    $container = $('#new-branch');
    // If the user doesn't have the required permissions the container isn't
    // rendered at all.
-    if ($container.length === 0) {
+    if (Issue.$btnNewBranch.length === 0) {
      return;
    }
-    return $.getJSON($container.data('path')).error(function() {
-      $container.find('.unavailable').show();
-      return new Flash('Failed to check if a new branch can be created.', 'alert');
-    }).success(function(data) {
-      if (data.can_create_branch) {
-        $container.find('.available').show();
-      } else {
-        return $container.find('.unavailable').show();
-      }
+    return $.getJSON(Issue.$btnNewBranch.data('path')).fail(function() {
+      Issue.setNewBranchButtonState(false, false);
+      new Flash('Failed to check if a new branch can be created.');
+    }).done(function(data) {
+      Issue.setNewBranchButtonState(false, data.can_create_branch);
    });
  }
+
+  static setNewBranchButtonState(isPending, canCreate) {
+    if (Issue.$btnNewBranch.length === 0) {
+      return;
+    }
+
+    Issue.$btnNewBranch.find('.available').toggle(!isPending && canCreate);
+    Issue.$btnNewBranch.find('.unavailable').toggle(!isPending && !canCreate);
+  }
 }

 export default Issue;
--- a/app/assets/stylesheets/framework/calendar.scss
+++ b/app/assets/stylesheets/framework/calendar.scss
@ -5,7 +5,7 @@
  direction: rtl;

  @media (min-width: $screen-sm-min) and (max-width: $screen-md-max) {
-    overflow-x: scroll;
+    overflow-x: auto;
  }
 }

--- a/app/assets/stylesheets/framework/filters.scss
+++ b/app/assets/stylesheets/framework/filters.scss
@ -82,7 +82,7 @@
  .input-token:last-child {
    flex: 1;
    -webkit-flex: 1;
-    max-width: initial;
+    max-width: inherit;
  }
 }

@ -246,17 +246,17 @@
  }
 }

-.filtered-search-history-dropdown-toggle-button {
+.filtered-search-history-dropdown-wrapper {
+  position: static;
  display: flex;
-  align-items: center;
-  width: auto;
-  height: 100%;
-  padding-top: 0;
-  padding-left: 0.75em;
-  padding-bottom: 0;
-  padding-right: 0.5em;
+  flex-direction: column;
+}
+
+.filtered-search-history-dropdown-toggle-button {
+  flex: 1;
+  width: auto;
+  padding-right: 10px;

-  background-color: transparent;
  border-radius: 0;
  border-top: 0;
  border-left: 0;
@ -264,6 +264,7 @@
  border-right: 1px solid $border-color;

  color: $gl-text-color-secondary;
+  line-height: 1;

  transition: color 0.1s linear;

@ -275,24 +276,21 @@
  }

  .dropdown-toggle-text {
+    display: inline-block;
    color: inherit;

    .fa {
+      vertical-align: middle;
      color: inherit;
    }
  }

  .fa {
-    position: initial;
+    position: static;
  }

 }

-.filtered-search-history-dropdown-wrapper {
-  position: initial;
-  flex-shrink: 0;
-}
-
 .filtered-search-history-dropdown {
  width: 40%;

--- a/app/assets/stylesheets/pages/builds.scss
+++ b/app/assets/stylesheets/pages/builds.scss
@ -39,7 +39,7 @@
    overflow-y: hidden;
    font-size: 12px;

-    .fa-refresh {
+    .fa-spinner {
      font-size: 24px;
      margin-left: 20px;
    }
@ -219,7 +219,7 @@
  font-size: 12px;
  position: relative;

-  .fa-refresh {
+  .fa-spinner {
    font-size: 24px;
  }

@ -366,7 +366,7 @@
        background-color: $row-hover;
      }

-      .fa-refresh {
+      .fa-spinner {
        font-size: 13px;
        margin-left: 3px;
      }
--- a/app/assets/stylesheets/pages/events.scss
+++ b/app/assets/stylesheets/pages/events.scss
@ -10,10 +10,14 @@
  position: relative;

  &.event-inline {
-    .profile-icon {
+    .system-note-image {
      top: 20px;
    }

+    .user-avatar {
+      top: 14px;
+    }
+
    .event-title,
    .event-item-timestamp {
      line-height: 40px;
@ -24,7 +28,7 @@
    color: $gl-text-color;
  }

-  .profile-icon {
+  .system-note-image {
    position: absolute;
    left: 0;
    top: 14px;
@ -35,15 +39,18 @@
      fill: $gl-text-color-secondary;
    }

-    &.open-icon svg {
-      fill: $green-300;
+    &.opened-icon,
+    &.created-icon {
+      svg {
+        fill: $green-300;
+      }
    }

    &.closed-icon svg {
      fill: $red-300;
    }

-    &.fork-icon svg {
+    &.accepted-icon svg {
      fill: $blue-300;
    }
  }
@ -128,8 +135,7 @@
    li {
      &.commit {
        background: transparent;
-        padding: 3px;
-        padding-left: 0;
+        padding: 0;
        border: none;

        .commit-row-title {
@ -183,7 +189,7 @@
      max-width: 100%;
    }

-    .profile-icon {
+    .system-note-image {
      display: none;
    }

--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@ -527,6 +527,8 @@
  }

  .comments-disabled-notif {
+    line-height: 28px;
+
    .btn {
      margin-left: 5px;
    }
--- a/app/assets/stylesheets/pages/notes.scss
+++ b/app/assets/stylesheets/pages/notes.scss
@ -18,12 +18,12 @@ ul.notes {
    float: left;

    svg {
-      width: 18px;
-      height: 18px;
+      width: 16px;
+      height: 16px;
      fill: $gray-darkest;
      position: absolute;
-      left: 30px;
-      top: 15px;
+      left: 0;
+      top: 16px;
    }
  }

@ -144,6 +144,10 @@ ul.notes {
    padding: 0;
    clear: both;

+    @media (min-width: $screen-sm-min) {
+      margin-left: 65px;
+    }
+
    &.timeline-entry::after {
      clear: none;
    }
@ -172,6 +176,10 @@ ul.notes {

    .timeline-content {
      padding: 14px 10px;
+
+      @media (min-width: $screen-sm-min) {
+        margin-left: 20px;
+      }
    }

    .note-header {
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@ -61,7 +61,6 @@ class Projects::CompareController < Projects::ApplicationController
      @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last

      @diff_notes_disabled = true
-      @grouped_diff_discussions = {}
    end
  end

--- a/app/controllers/projects/hooks_controller.rb
+++ b/app/controllers/projects/hooks_controller.rb
@ -10,7 +10,7 @@ class Projects::HooksController < Projects::ApplicationController
    @hook = @project.hooks.new(hook_params)
    @hook.save

-    unless @hook.valid?      
+    unless @hook.valid?
      @hooks = @project.hooks.select(&:persisted?)
      flash[:alert] = @hook.errors.full_messages.join.html_safe
    end
@ -49,7 +49,7 @@ class Projects::HooksController < Projects::ApplicationController

  def hook_params
    params.require(:hook).permit(
-      :build_events,
+      :job_events,
      :pipeline_events,
      :enable_ssl_verification,
      :issues_events,
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@ -16,7 +16,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
  before_action :define_show_vars, only: [:show, :diffs, :commits, :conflicts, :conflict_for_path, :builds, :pipelines]
  before_action :define_widget_vars, only: [:merge, :cancel_merge_when_pipeline_succeeds, :merge_check]
  before_action :define_commit_vars, only: [:diffs]
-  before_action :define_diff_comment_vars, only: [:diffs]
  before_action :ensure_ref_fetched, only: [:show, :diffs, :commits, :builds, :conflicts, :conflict_for_path, :pipelines]
  before_action :close_merge_request_without_source_project, only: [:show, :diffs, :commits, :builds, :pipelines]
  before_action :apply_diff_view_cookie!, only: [:new_diffs]
@ -39,7 +38,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
    @collection_type    = "MergeRequest"
    @merge_requests     = merge_requests_collection
    @merge_requests     = @merge_requests.page(params[:page])
-    @merge_requests     = @merge_requests.includes(merge_request_diff: :merge_request)
+    @merge_requests     = @merge_requests.preload(merge_request_diff: :merge_request)
    @issuable_meta_data = issuable_meta_data(@merge_requests, @collection_type)

    if @merge_requests.out_of_range? && @merge_requests.total_pages != 0
@ -101,34 +100,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController
    respond_to do |format|
      format.html { define_discussion_vars }
      format.json do
-        @merge_request_diff =
-          if params[:diff_id]
-            @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
-          else
-            @merge_request.merge_request_diff
-          end
-
-        @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
-        @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
-
-        if params[:start_sha].present?
-          @start_sha = params[:start_sha]
-          @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
-
-          unless @start_version
-            @start_sha = @merge_request_diff.head_commit_sha
-            @start_version = @merge_request_diff
-          end
-        end
+        define_diff_vars
+        define_diff_comment_vars

        @environment = @merge_request.environments_for(current_user).last

-        if @start_sha
-          compared_diff_version
-        else
-          original_diff_version
-        end
-
        render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") }
      end
    end
@ -140,16 +116,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController
  def diff_for_path
    if params[:id]
      merge_request
+      define_diff_vars
      define_diff_comment_vars
    else
      build_merge_request
+      @diffs = @merge_request.diffs(diff_options)
      @diff_notes_disabled = true
-      @grouped_diff_discussions = {}
    end

    define_commit_vars

-    render_diff_for_path(@merge_request.diffs(diff_options))
+    render_diff_for_path(@diffs)
  end

  def commits
@ -586,15 +563,46 @@ class Projects::MergeRequestsController < Projects::ApplicationController
    @base_commit = @merge_request.diff_base_commit || @merge_request.likely_diff_base_commit
  end

+  def define_diff_vars
+    @merge_request_diff =
+      if params[:diff_id]
+        @merge_request.merge_request_diffs.viewable.find(params[:diff_id])
+      else
+        @merge_request.merge_request_diff
+      end
+
+    @merge_request_diffs = @merge_request.merge_request_diffs.viewable.select_without_diff
+    @comparable_diffs = @merge_request_diffs.select { |diff| diff.id < @merge_request_diff.id }
+
+    if params[:start_sha].present?
+      @start_sha = params[:start_sha]
+      @start_version = @comparable_diffs.find { |diff| diff.head_commit_sha == @start_sha }
+
+      unless @start_version
+        @start_sha = @merge_request_diff.head_commit_sha
+        @start_version = @merge_request_diff
+      end
+    end
+
+    @diffs =
+      if @start_sha
+        @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
+      else
+        @merge_request_diff.diffs(diff_options)
+      end
+  end
+
  def define_diff_comment_vars
    @new_diff_note_attrs = {
      noteable_type: 'MergeRequest',
      noteable_id: @merge_request.id
    }

+    @diff_notes_disabled = !@merge_request_diff.latest? || @start_sha
+
    @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs?

-    @grouped_diff_discussions = @merge_request.grouped_diff_discussions
+    @grouped_diff_discussions = @merge_request.grouped_diff_discussions(@merge_request_diff.diff_refs)
    @notes = prepare_notes_for_rendering(@grouped_diff_discussions.values.flatten.flat_map(&:notes))
  end

@ -678,16 +686,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController
    @merge_request = MergeRequests::BuildService.new(project, current_user, merge_request_params.merge(diff_options: diff_options)).execute
  end

-  def compared_diff_version
-    @diff_notes_disabled = true
-    @diffs = @merge_request_diff.compare_with(@start_sha).diffs(diff_options)
-  end
-
-  def original_diff_version
-    @diff_notes_disabled = !@merge_request_diff.latest?
-    @diffs = @merge_request_diff.diffs(diff_options)
-  end
-
  def close_merge_request_without_source_project
    if !@merge_request.source_project && @merge_request.open?
      @merge_request.close
--- a/app/helpers/diff_helper.rb
+++ b/app/helpers/diff_helper.rb
@ -62,6 +62,8 @@ module DiffHelper
  end

  def parallel_diff_discussions(left, right, diff_file)
+    return unless @grouped_diff_discussions
+    
    discussions_left = discussions_right = nil

    if left && (left.unchanged? || left.removed?)
--- a/app/helpers/events_helper.rb
+++ b/app/helpers/events_helper.rb
@ -1,4 +1,15 @@
 module EventsHelper
+  ICON_NAMES_BY_EVENT_TYPE = {
+    'pushed to' => 'icon_commit',
+    'pushed new' => 'icon_commit',
+    'created' => 'icon_status_open',
+    'opened' => 'icon_status_open',
+    'closed' => 'icon_status_closed',
+    'accepted' => 'icon_code_fork',
+    'commented on' => 'icon_comment_o',
+    'deleted' => 'icon_trash_o'
+  }.freeze
+
  def link_to_author(event)
    author = event.author

@ -183,4 +194,21 @@ module EventsHelper
      "event-inline"
    end
  end
+
+  def icon_for_event(note)
+    icon_name = ICON_NAMES_BY_EVENT_TYPE[note]
+    custom_icon(icon_name) if icon_name
+  end
+
+  def icon_for_profile_event(event)
+    if current_path?('users#show')
+      content_tag :div, class: "system-note-image #{event.action_name.parameterize}-icon" do
+        icon_for_event(event.action_name)
+      end
+    else
+      content_tag :div, class: 'system-note-image user-avatar' do
+        author_avatar(event, size: 32)
+      end
+    end
+  end
 end
--- a/app/helpers/javascript_helper.rb
+++ b/app/helpers/javascript_helper.rb
@ -3,7 +3,8 @@ module JavascriptHelper
    javascript_include_tag asset_path(js)
  end

-  def page_specific_javascript_bundle_tag(js)
-    javascript_include_tag(*webpack_asset_paths(js))
+  # deprecated; use webpack_bundle_tag directly instead
+  def page_specific_javascript_bundle_tag(bundle)
+    webpack_bundle_tag(bundle)
  end
 end
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@ -61,12 +61,23 @@ module NotesHelper
  end

  def discussion_diff_path(discussion)
-    return unless discussion.diff_discussion?
+    if discussion.for_merge_request? && discussion.diff_discussion?
+      if discussion.active?
+        # Without a diff ID, the link always points to the latest diff version
+        diff_id = nil
+      elsif merge_request_diff = discussion.latest_merge_request_diff
+        diff_id = merge_request_diff.id
+      else
+        # If the discussion is not active, and we cannot find the latest
+        # merge request diff for this discussion, we return no path at all.
+        return
+      end

-    if discussion.for_merge_request? && discussion.active?
-      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+      diffs_namespace_project_merge_request_path(discussion.project.namespace, discussion.project, discussion.noteable, diff_id: diff_id, anchor: discussion.line_code)
    elsif discussion.for_commit?
-      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: discussion.line_code)
+      anchor = discussion.line_code if discussion.diff_discussion?
+
+      namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor)
    end
  end
 end
--- a/app/helpers/webpack_helper.rb
+++ b/app/helpers/webpack_helper.rb
@ -0,0 +1,30 @@
+require 'webpack/rails/manifest'
+
+module WebpackHelper
+  def webpack_bundle_tag(bundle)
+    javascript_include_tag(*gitlab_webpack_asset_paths(bundle))
+  end
+
+  # override webpack-rails gem helper until changes can make it upstream
+  def gitlab_webpack_asset_paths(source, extension: nil)
+    return "" unless source.present?
+
+    paths = Webpack::Rails::Manifest.asset_paths(source)
+    if extension
+      paths = paths.select { |p| p.ends_with? ".#{extension}" }
+    end
+
+    # include full webpack-dev-server url for rspec tests running locally
+    if Rails.env.test? && Rails.configuration.webpack.dev_server.enabled
+      host = Rails.configuration.webpack.dev_server.host
+      port = Rails.configuration.webpack.dev_server.port
+      protocol = Rails.configuration.webpack.dev_server.https ? 'https' : 'http'
+
+      paths.map! do |p|
+        "#{protocol}://#{host}:#{port}#{p}"
+      end
+    end
+
+    paths
+  end
+end
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@ -326,14 +326,13 @@ class Commit
  end

  def raw_diffs(*args)
-    use_gitaly = Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
-    deltas_only = args.last.is_a?(Hash) && args.last[:deltas_only]
-
-    if use_gitaly && !deltas_only
-      Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
-    else
-      raw.diffs(*args)
-    end
+    # NOTE: This feature is intentionally disabled until
+    # https://gitlab.com/gitlab-org/gitaly/issues/178 is resolved
+    # if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
+    #   Gitlab::GitalyClient::Commit.diff_from_parent(self, *args)
+    # else
+    raw.diffs(*args)
+    # end
  end

  def diffs(diff_options = nil)
--- a/app/models/concerns/discussion_on_diff.rb
+++ b/app/models/concerns/discussion_on_diff.rb
@ -2,11 +2,9 @@
 module DiscussionOnDiff
  extend ActiveSupport::Concern

+  NUMBER_OF_TRUNCATED_DIFF_LINES = 16
+
  included do
-    NUMBER_OF_TRUNCATED_DIFF_LINES = 16
-
-    memoized_values << :active
-
    delegate  :line_code,
              :original_line_code,
              :diff_file,
@ -29,12 +27,6 @@ module DiscussionOnDiff
    true
  end

-  def active?
-    return @active if @active.present?
-
-    @active = first_note.active?
-  end
-
  # Returns an array of at most 16 highlighted lines above a diff note
  def truncated_diff_lines(highlight: true)
    lines = highlight ? highlighted_diff_lines : diff_lines
--- a/app/models/concerns/note_on_diff.rb
+++ b/app/models/concerns/note_on_diff.rb
@ -25,4 +25,18 @@ module NoteOnDiff
  def diff_attributes
    raise NotImplementedError
  end
+
+  def active?(diff_refs = nil)
+    raise NotImplementedError
+  end
+
+  private
+
+  def noteable_diff_refs
+    if noteable.respond_to?(:diff_sha_refs)
+      noteable.diff_sha_refs
+    else
+      noteable.diff_refs
+    end
+  end
 end
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@ -36,10 +36,10 @@ module Noteable
      .discussions(self)
  end

-  def grouped_diff_discussions
+  def grouped_diff_discussions(*args)
    # Doesn't use `discussion_notes`, because this may include commit diff notes
    # besides MR diff notes, that we do no want to display on the MR Changes tab.
-    notes.inc_relations_for_view.grouped_diff_discussions
+    notes.inc_relations_for_view.grouped_diff_discussions(*args)
  end

  def resolvable_discussions
--- a/app/models/diff_discussion.rb
+++ b/app/models/diff_discussion.rb
@ -10,6 +10,7 @@ class DiffDiscussion < Discussion

  delegate  :position,
            :original_position,
+            :latest_merge_request_diff,

            to: :first_note

--- a/app/models/diff_note.rb
+++ b/app/models/diff_note.rb
@ -65,20 +65,18 @@ class DiffNote < Note
    self.position.diff_refs == diff_refs
  end

+  def latest_merge_request_diff
+    return unless for_merge_request?
+
+    self.noteable.merge_request_diff_for(self.position.diff_refs)
+  end
+
  private

  def supported?
    for_commit? || self.noteable.has_complete_diff_refs?
  end

-  def noteable_diff_refs
-    if noteable.respond_to?(:diff_sha_refs)
-      noteable.diff_sha_refs
-    else
-      noteable.diff_refs
-    end
-  end
-
  def set_original_position
    self.original_position = self.position.dup unless self.original_position&.complete?
  end
--- a/app/models/label.rb
+++ b/app/models/label.rb
@ -21,6 +21,8 @@ class Label < ActiveRecord::Base
  has_many :issues, through: :label_links, source: :target, source_type: 'Issue'
  has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest'

+  before_validation :strip_whitespace_from_title_and_color
+
  validates :color, color: true, allow_blank: false

  # Don't allow ',' for label titles
@ -193,4 +195,8 @@ class Label < ActiveRecord::Base
  def sanitize_title(value)
    CGI.unescapeHTML(Sanitize.clean(value.to_s))
  end
+
+  def strip_whitespace_from_title_and_color
+    %w(color title).each { |attr| self[attr] = self[attr]&.strip }
+  end
 end
--- a/app/models/legacy_diff_discussion.rb
+++ b/app/models/legacy_diff_discussion.rb
@ -7,6 +7,8 @@
 class LegacyDiffDiscussion < Discussion
  include DiscussionOnDiff

+  memoized_values << :active
+
  def legacy_diff_discussion?
    true
  end
@ -15,6 +17,12 @@ class LegacyDiffDiscussion < Discussion
    LegacyDiffNote
  end

+  def active?(*args)
+    return @active if @active.present?
+
+    @active = first_note.active?(*args)
+  end
+
  def collapsed?
    !active?
  end
--- a/app/models/legacy_diff_note.rb
+++ b/app/models/legacy_diff_note.rb
@ -56,11 +56,12 @@ class LegacyDiffNote < Note
  #
  # If the note's current diff cannot be matched in the MergeRequest's current
  # diff, it's considered inactive.
-  def active?
+  def active?(diff_refs = nil)
    return @active if defined?(@active)
    return true if for_commit?
    return true unless diff_line
    return false unless noteable
+    return false if diff_refs && diff_refs != noteable_diff_refs

    noteable_diff = find_noteable_diff

--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@ -366,6 +366,14 @@ class MergeRequest < ActiveRecord::Base
    merge_request_diff(true)
  end

+  def merge_request_diff_for(diff_refs)
+    @merge_request_diffs_by_diff_refs ||= Hash.new do |h, diff_refs|
+      h[diff_refs] = merge_request_diffs.viewable.select_without_diff.find_by_diff_refs(diff_refs)
+    end
+
+    @merge_request_diffs_by_diff_refs[diff_refs]
+  end
+
  def reload_diff_if_branch_changed
    if source_branch_changed? || target_branch_changed?
      reload_diff
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@ -31,6 +31,10 @@ class MergeRequestDiff < ActiveRecord::Base
  # It allows you to override variables like head_commit_sha before getting diff.
  after_create :save_git_content, unless: :importing?

+  def self.find_by_diff_refs(diff_refs)
+    find_by(start_commit_sha: diff_refs.start_sha, head_commit_sha: diff_refs.head_sha, base_commit_sha: diff_refs.base_sha)
+  end
+
  def self.select_without_diff
    select(column_names - ['st_diffs'])
  end
@ -130,6 +134,12 @@ class MergeRequestDiff < ActiveRecord::Base
    st_commits.map { |commit| commit[:id] }
  end

+  def diff_refs=(new_diff_refs)
+    self.base_commit_sha = new_diff_refs&.base_sha
+    self.start_commit_sha = new_diff_refs&.start_sha
+    self.head_commit_sha = new_diff_refs&.head_sha
+  end
+
  def diff_refs
    return unless start_commit_sha || base_commit_sha

--- a/app/models/note.rb
+++ b/app/models/note.rb
@ -96,6 +96,7 @@ class Note < ActiveRecord::Base
  before_validation :set_discussion_id, on: :create
  after_save :keep_around_commit, unless: :for_personal_snippet?
  after_save :expire_etag_cache
+  after_destroy :expire_etag_cache

  class << self
    def model_name
@ -113,11 +114,11 @@ class Note < ActiveRecord::Base
      Discussion.build(notes)
    end

-    def grouped_diff_discussions
+    def grouped_diff_discussions(diff_refs = nil)
      diff_notes.
        fresh.
        discussions.
-        select(&:active?).
+        select { |n| n.active?(diff_refs) }.
        group_by(&:line_code)
    end

@ -140,6 +141,10 @@ class Note < ActiveRecord::Base
    true
  end

+  def latest_merge_request_diff
+    nil
+  end
+
  def max_attachment_size
    current_application_settings.max_attachment_size.megabytes.to_i
  end
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@ -963,13 +963,15 @@ class Repository
  end

  def is_ancestor?(ancestor_id, descendant_id)
-    Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
-      if is_enabled
-        raw_repository.is_ancestor?(ancestor_id, descendant_id)
-      else
-        merge_base_commit(ancestor_id, descendant_id) == ancestor_id
-      end
-    end
+    # NOTE: This feature is intentionally disabled until
+    # https://gitlab.com/gitlab-org/gitlab-ce/issues/30586 is resolved
+    # Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
+    #   if is_enabled
+    #     raw_repository.is_ancestor?(ancestor_id, descendant_id)
+    #   else
+    merge_base_commit(ancestor_id, descendant_id) == ancestor_id
+    #   end
+    # end
  end

  def empty_repo?
--- a/app/policies/group_policy.rb
+++ b/app/policies/group_policy.rb
@ -28,6 +28,7 @@ class GroupPolicy < BasePolicy
      can! :admin_namespace
      can! :admin_group_member
      can! :change_visibility_level
+      can! :create_subgroup if @user.can_create_group
    end

    if globally_viewable && @subject.request_access_enabled && !member
--- a/app/views/admin/health_check/show.html.haml
+++ b/app/views/admin/health_check/show.html.haml
@ -13,7 +13,7 @@
    = button_to reset_health_check_token_admin_application_settings_path,
      method: :put, class: 'btn btn-default',
      data: { confirm: 'Are you sure you want to reset the health check token?' } do
-      = icon('refresh')
+      = icon('spinner')
      Reset health check access token
  %p.light
    Health information can be retrieved as plain text, JSON, or XML using:
--- a/app/views/admin/runners/index.html.haml
+++ b/app/views/admin/runners/index.html.haml
@ -21,7 +21,7 @@
        = button_to reset_runners_token_admin_application_settings_path,
          method: :put, class: 'btn btn-default',
          data: { confirm: 'Are you sure you want to reset registration token?' } do
-          = icon('refresh')
+          = icon('spinner')
          Reset runners registration token

  .bs-callout
--- a/app/views/discussions/_discussion.html.haml
+++ b/app/views/discussions/_discussion.html.haml
@ -20,21 +20,22 @@
            = discussion.author.to_reference
            started a discussion

+            - url = discussion_diff_path(discussion)
            - if discussion.for_commit? && @noteable != discussion.noteable
              on
              - commit = discussion.noteable
              - if commit
                commit
-                - anchor = discussion.line_code if discussion.diff_discussion?
-                = link_to commit.short_id, namespace_project_commit_path(discussion.project.namespace, discussion.project, discussion.noteable, anchor: anchor), class: 'monospace'
+                = link_to commit.short_id, url, class: 'monospace'
              - else
                a deleted commit
            - elsif discussion.diff_discussion?
              on
-              - if discussion.active?
-                = link_to 'the diff', discussion_diff_path(discussion)
-              - else
-                an outdated diff
+              = conditional_link_to url.present?, url do
+                - if discussion.active?
+                  the diff
+                - else
+                  an outdated diff

            = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago")
          = render "discussions/headline", discussion: discussion
--- a/app/views/events/event/_common.html.haml
+++ b/app/views/events/event/_common.html.haml
@ -1,13 +1,4 @@
- if event.target
-  - if event.action_name == "opened"
-    .profile-icon.open-icon
-      = custom_icon("icon_status_open")
-  - elsif event.action_name == "closed"
-    .profile-icon.closed-icon
-      = custom_icon("icon_status_closed")
-  - else
-    .profile-icon.fork-icon
-      = custom_icon("icon_code_fork")
+= icon_for_profile_event(event)

 .event-title
  %span.author_name= link_to_author event
--- a/app/views/events/event/_created_project.html.haml
+++ b/app/views/events/event/_created_project.html.haml
@ -1,5 +1,4 @@
-.profile-icon.open-icon
-  = custom_icon("icon_status_open")
+= icon_for_profile_event(event)

 .event-title
  %span.author_name= link_to_author event
--- a/app/views/events/event/_note.html.haml
+++ b/app/views/events/event/_note.html.haml
@ -1,5 +1,4 @@
-.profile-icon
-  = custom_icon("icon_comment_o")
+= icon_for_profile_event(event)

 .event-title
  %span.author_name= link_to_author event
--- a/app/views/events/event/_push.html.haml
+++ b/app/views/events/event/_push.html.haml
@ -1,10 +1,6 @@
 - project = event.project

-.profile-icon
-  - if event.action_name == "deleted"
-    = custom_icon("trash_o")
-  - else
-    = custom_icon("icon_commit")
+= icon_for_profile_event(event)

 .event-title
  %span.author_name= link_to_author event
--- a/app/views/groups/subgroups.html.haml
+++ b/app/views/groups/subgroups.html.haml
@ -9,7 +9,7 @@
    .nav-controls
      = form_tag request.path, method: :get do |f|
        = search_field_tag :filter_groups, params[:filter_groups], placeholder: 'Filter by name', class: 'form-control', spellcheck: false
-      - if can? current_user, :admin_group, @group
+      - if can?(current_user, :create_subgroup, @group)
        = link_to new_group_path(parent_id: @group.id), class: 'btn btn-new pull-right' do
          New Subgroup

--- a/app/views/help/ui.html.haml
+++ b/app/views/help/ui.html.haml
@ -252,7 +252,7 @@
          = icon('chevron-down')
        .dropdown-menu.dropdown-select.dropdown-menu-selectable
          .dropdown-title
-            %span Dropdown Title
+            %span Dropdown title
            %button.dropdown-title-button.dropdown-menu-close{ aria: { label: "Close" } }
              = icon('times')
          .dropdown-input
@ -291,7 +291,7 @@
          = icon('chevron-down')
        .dropdown-menu.dropdown-select.dropdown-menu-selectable.is-loading
          .dropdown-title
-            %span Dropdown Title
+            %span Dropdown title
            %button.dropdown-title-button.dropdown-menu-close{ aria: { label: "Close" } }
              = icon('times')
          .dropdown-input
@ -335,7 +335,7 @@
          = icon('chevron-down')
        .dropdown-menu.dropdown-select.dropdown-menu-selectable.dropdown-menu-user
          .dropdown-title
-            %span Dropdown Title
+            %span Dropdown title
            %button.dropdown-title-button.dropdown-menu-close{ aria: { label: "Close" } }
              = icon('times')
          .dropdown-input
@ -362,7 +362,7 @@
            .dropdown-title
              %button.dropdown-title-button.dropdown-menu-back{ aria: { label: "Go back" } }
                = icon('arrow-left')
-              %span Dropdown Title
+              %span Dropdown title
              %button.dropdown-title-button.dropdown-menu-close{ aria: { label: "Close" } }
                = icon('times')
            .dropdown-input
--- a/app/views/layouts/_head.html.haml
+++ b/app/views/layouts/_head.html.haml
@ -28,9 +28,9 @@
  = stylesheet_link_tag "application", media: "all"
  = stylesheet_link_tag "print",       media: "print"

-  = javascript_include_tag(*webpack_asset_paths("runtime"))
-  = javascript_include_tag(*webpack_asset_paths("common"))
-  = javascript_include_tag(*webpack_asset_paths("main"))
+  = webpack_bundle_tag "runtime"
+  = webpack_bundle_tag "common"
+  = webpack_bundle_tag "main"

  - if content_for?(:page_specific_javascripts)
    = yield :page_specific_javascripts
--- a/app/views/projects/blob/_blob.html.haml
+++ b/app/views/projects/blob/_blob.html.haml
@ -25,13 +25,6 @@
 #blob-content-holder.blob-content-holder
  %article.file-holder
    = render "projects/blob/header", blob: blob
-    - if current_user
-      .js-file-fork-suggestion-section.file-fork-suggestion.hidden
-        %span.file-fork-suggestion-note
-          You don't have permission to edit this file. Try forking this project to edit the file.
-        = link_to 'Fork', fork_path, method: :post, class: 'btn btn-grouped btn-inverted btn-new'
-        %button.js-cancel-fork-suggestion.btn.btn-grouped{ type: 'button' }
-          Cancel

    - if blob.empty?
      .file-content.code
--- a/app/views/projects/blob/_header.html.haml
+++ b/app/views/projects/blob/_header.html.haml
@ -38,3 +38,10 @@
      - if current_user
        = replace_blob_link
        = delete_blob_link
+- if current_user
+  .js-file-fork-suggestion-section.file-fork-suggestion.hidden
+    %span.file-fork-suggestion-note
+      You don't have permission to edit this file. Try forking this project to edit the file.
+    = link_to 'Fork', fork_path, method: :post, class: 'btn btn-grouped btn-inverted btn-new'
+    %button.js-cancel-fork-suggestion.btn.btn-grouped{ type: 'button' }
+      Cancel
--- a/app/views/projects/builds/_sidebar.html.haml
+++ b/app/views/projects/builds/_sidebar.html.haml
@ -136,7 +136,7 @@
              - else
                = build.id
            - if build.retried?
-              %i.fa.fa-refresh.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' }
+              %i.fa.fa-spinner.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' }

 :javascript
  new Sidebar();
--- a/app/views/projects/ci/builds/_build.html.haml
+++ b/app/views/projects/ci/builds/_build.html.haml
@ -36,7 +36,7 @@
      = icon('warning', class: 'text-warning has-tooltip', title: 'Job is stuck. Check runners.')

    - if retried
-      = icon('refresh', class: 'text-warning has-tooltip', title: 'Job was retried')
+      = icon('spinner', class: 'text-warning has-tooltip', title: 'Job was retried')

    .label-container
      - if job.tags.any?
--- a/app/views/projects/diffs/_parallel_view.html.haml
+++ b/app/views/projects/diffs/_parallel_view.html.haml
@ -5,8 +5,7 @@
      - left = line[:left]
      - right = line[:right]
      - last_line = right.new_pos if right
-      - unless @diff_notes_disabled
-        - discussions_left, discussions_right = parallel_diff_discussions(left, right, diff_file)
+      - discussions_left, discussions_right = parallel_diff_discussions(left, right, diff_file)
      %tr.line_holder.parallel
        - if left
          - case left.type
--- a/app/views/projects/diffs/_text_file.html.haml
+++ b/app/views/projects/diffs/_text_file.html.haml
@ -4,11 +4,10 @@
    %a.show-suppressed-diff.js-show-suppressed-diff Changes suppressed. Click to show.

 %table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' }
-  - discussions = @grouped_diff_discussions unless @diff_notes_disabled
  = render partial: "projects/diffs/line",
    collection: diff_file.highlighted_diff_lines,
    as: :line,
-    locals: { diff_file: diff_file, discussions: discussions }
+    locals: { diff_file: diff_file, discussions: @grouped_diff_discussions }

  - if !diff_file.new_file && !diff_file.deleted_file && diff_file.highlighted_diff_lines.any?
    - last_line = diff_file.highlighted_diff_lines.last
--- a/app/views/projects/merge_requests/show/_versions.html.haml
+++ b/app/views/projects/merge_requests/show/_versions.html.haml
@ -72,13 +72,16 @@
        = link_to namespace_project_compare_path(@project.namespace, @project, from: @start_version.base_commit_sha, to: @merge_request_diff.base_commit_sha) do
          new commits
        from
-        %code= @merge_request.target_branch
+        = succeed '.' do
+          %code= @merge_request.target_branch

-    - unless @merge_request_diff.latest? && !@start_sha
+    - if @diff_notes_disabled
      .comments-disabled-notif.content-block
        = icon('info-circle')
        - if @start_sha
          Comments are disabled because you're comparing two versions of this merge request.
        - else
-          Comments are disabled because you're viewing an old version of this merge request.
-        = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'
+          Discussions on this version of the merge request are displayed but comment creation is disabled.
+
+        .pull-right
+          = link_to 'Show latest version', diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), class: 'btn btn-sm'
--- a/app/views/shared/icons/_icon_merged.svg
+++ b/app/views/shared/icons/_icon_merged.svg
@ -1 +1 @@
-<svg width="14" height="14" viewBox="0 0 14 14" xmlns="http://www.w3.org/2000/svg"><path d="M0 7c0-3.866 3.142-7 7-7 3.866 0 7 3.142 7 7 0 3.866-3.142 7-7 7-3.866 0-7-3.142-7-7z"/><path d="M1 7c0 3.309 2.69 6 6 6 3.309 0 6-2.69 6-6 0-3.309-2.69-6-6-6-3.309 0-6 2.69-6 6z" fill="#FFF"/><path d="M9.427 6.523a.932.932 0 0 0-.808.489v-.01c-.49-.01-1.059-.172-1.46-.489-.35-.278-.7-.772-.882-1.17a.964.964 0 0 0 .35-.744.943.943 0 0 0-.934-.959c-.518 0-.933.432-.933.964 0 .35.191.662.467.825v3.147a.97.97 0 0 0-.467.825c0 .532.415.959.933.959a.943.943 0 0 0 .934-.96.965.965 0 0 0-.467-.824V6.844c.313.336.672.61 1.073.81.402.202.948.303 1.386.308v-.01c.168.293.467.49.808.49a.943.943 0 0 0 .933-.96.943.943 0 0 0-.933-.96z"/></svg>
+<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16"><path d="m2 3c.552 0 1-.448 1-1 0-.552-.448-1-1-1-.552 0-1 .448-1 1 0 .552.448 1 1 1m.761.85c.154 2.556 1.987 4.692 4.45 5.255.328-.655 1.01-1.105 1.789-1.105 1.105 0 2 .895 2 2 0 1.105-.895 2-2 2-.89 0-1.645-.582-1.904-1.386-1.916-.376-3.548-1.5-4.596-3.044v4.493c.863.222 1.5 1.01 1.5 1.937 0 1.105-.895 2-2 2-1.105 0-2-.895-2-2 0-.74.402-1.387 1-1.732v-8.535c-.598-.346-1-.992-1-1.732 0-1.105.895-2 2-2 1.105 0 2 .895 2 2 0 .835-.512 1.551-1.239 1.85m6.239 7.15c.552 0 1-.448 1-1 0-.552-.448-1-1-1-.552 0-1 .448-1 1 0 .552.448 1 1 1m-7 4c.552 0 1-.448 1-1 0-.552-.448-1-1-1-.552 0-1 .448-1 1 0 .552.448 1 1 1" transform="translate(3)"/></svg>
--- a/app/views/shared/icons/_icon_trash_o.svg
+++ b/app/views/shared/icons/_icon_trash_o.svg
--- a/app/workers/build_coverage_worker.rb
+++ b/app/workers/build_coverage_worker.rb
@ -3,7 +3,6 @@ class BuildCoverageWorker
  include BuildQueue

  def perform(build_id)
-    Ci::Build.find_by(id: build_id)
-      .try(:update_coverage)
+    Ci::Build.find_by(id: build_id)&.update_coverage
  end
 end
--- a/changelogs/unreleased/2120-issues-search-bar-not-picking-up.yml
+++ b/changelogs/unreleased/2120-issues-search-bar-not-picking-up.yml
@ -0,0 +1,4 @@
+---
+title: Fix filtered search input width for IE
+merge_request:
+author:
--- a/changelogs/unreleased/22714-update-all-instances-of-fa-refresh.yml
+++ b/changelogs/unreleased/22714-update-all-instances-of-fa-refresh.yml
@ -0,0 +1,4 @@
+---
+title: Update all instances of the old loading icon
+merge_request: 10490
+author: Andrew Torres
--- a/changelogs/unreleased/27729-improve-webpack-dev-environment.yml
+++ b/changelogs/unreleased/27729-improve-webpack-dev-environment.yml
@ -0,0 +1,4 @@
+---
+title: Add webpack_bundle_tag helper to improve non-localhost GDK configurations
+merge_request: 10604
+author:
--- a/changelogs/unreleased/28017-separate-ce-params-on-api.yml
+++ b/changelogs/unreleased/28017-separate-ce-params-on-api.yml
@ -0,0 +1,4 @@
+---
+title: Separate CE params on Grape API
+merge_request:
+author:
--- a/changelogs/unreleased/29056-backport-ee-cleanup-database-file.yml
+++ b/changelogs/unreleased/29056-backport-ee-cleanup-database-file.yml
@ -0,0 +1,4 @@
+---
+title: Turns true value and false value database methods from instance to class methods
+merge_request: 10583
+author:
--- a/changelogs/unreleased/30457-expire-note-destroy.yml
+++ b/changelogs/unreleased/30457-expire-note-destroy.yml
@ -0,0 +1,4 @@
+---
+title: Fix issue's note cache expiration after delete
+merge_request:
+author: mhasbini
--- a/changelogs/unreleased/bb_save_trace.yml
+++ b/changelogs/unreleased/bb_save_trace.yml
@ -0,0 +1,5 @@
+---
+title: "[BB Importer] Save the error trace and the whole raw document to debug problems
+  easier"
+merge_request:
+author:
--- a/changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml
+++ b/changelogs/unreleased/dm-link-discussion-to-outdated-diff.yml
@ -0,0 +1,4 @@
+---
+title: Link to outdated diff in older MR version from outdated diff discussion
+merge_request:
+author:
--- a/changelogs/unreleased/fix-missing-capitalisation-buttons.yml
+++ b/changelogs/unreleased/fix-missing-capitalisation-buttons.yml
@ -0,0 +1,4 @@
+---
+title: Fix missing capitalisation on views
+merge_request:
+author:
--- a/changelogs/unreleased/fix-preloading-merge_request_diff.yml
+++ b/changelogs/unreleased/fix-preloading-merge_request_diff.yml
@ -0,0 +1,4 @@
+---
+title: Fix bad query for PostgreSQL showing merge requests list
+merge_request: 10666
+author:
--- a/changelogs/unreleased/fix_spaces_in_label_title.yml
+++ b/changelogs/unreleased/fix_spaces_in_label_title.yml
@ -0,0 +1,4 @@
+---
+title: Remove heading and trailing spaces from label's color and title
+merge_request: 10603
+author: blackst0ne
--- a/changelogs/unreleased/omnibus-gitlab-1993-check-shell-repositories-path-group-is-root.yml
+++ b/changelogs/unreleased/omnibus-gitlab-1993-check-shell-repositories-path-group-is-root.yml
@ -0,0 +1,4 @@
+---
+title: "Make the `gitlab:gitlab_shell:check` task check that the repositories storage path are owned by the `root` group"
+merge_request:
+author:
--- a/changelogs/unreleased/reset-new-branch-button.yml
+++ b/changelogs/unreleased/reset-new-branch-button.yml
@ -0,0 +1,4 @@
+---
+title: Reset New branch button when issue state changes
+merge_request: 5962
+author: winniehell
--- a/changelogs/unreleased/siemens-gitlab-ce-fix-subgroup-hide-button.yml
+++ b/changelogs/unreleased/siemens-gitlab-ce-fix-subgroup-hide-button.yml
@ -0,0 +1,4 @@
+---
+title: Hide new subgroup button if user has no permission to create one
+merge_request: 10627
+author:
--- a/changelogs/unreleased/user-activity-scroll-bar.yml
+++ b/changelogs/unreleased/user-activity-scroll-bar.yml
@ -0,0 +1,4 @@
+---
+title: Fix preemptive scroll bar on user activity calendar.
+merge_request: !10636
+author:
--- a/changelogs/unreleased/zj-api-fix-build-events.yml
+++ b/changelogs/unreleased/zj-api-fix-build-events.yml
@ -0,0 +1,4 @@
+---
+title: "Bugfix: POST /projects/:id/hooks and PUT /projects/:id/hook/:hook_id no longer ignore the the job_events param in the V4 API"
+merge_request: 10586
+author:
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@ -579,9 +579,9 @@ test:
    storages:
      default:
        path: tmp/tests/repositories/
-        gitaly_address: unix:<%= Rails.root.join('tmp/sockets/private/gitaly.socket') %>
+        gitaly_address: unix:tmp/tests/gitaly/gitaly.socket
  gitaly:
-    enabled: false
+    enabled: true
  backup:
    path: tmp/tests/backups
  gitlab_shell:
--- a/config/webpack.config.js
+++ b/config/webpack.config.js
@ -11,6 +11,7 @@ var WatchMissingNodeModulesPlugin = require('react-dev-utils/WatchMissingNodeMod
 var ROOT_PATH = path.resolve(__dirname, '..');
 var IS_PRODUCTION = process.env.NODE_ENV === 'production';
 var IS_DEV_SERVER = process.argv[1].indexOf('webpack-dev-server') !== -1;
+var DEV_SERVER_HOST = process.env.DEV_SERVER_HOST || 'localhost';
 var DEV_SERVER_PORT = parseInt(process.env.DEV_SERVER_PORT, 10) || 3808;
 var DEV_SERVER_LIVERELOAD = process.env.DEV_SERVER_LIVERELOAD !== 'false';
 var WEBPACK_REPORT = process.env.WEBPACK_REPORT;
@ -182,12 +183,13 @@ if (IS_PRODUCTION) {
 if (IS_DEV_SERVER) {
  config.devtool = 'cheap-module-eval-source-map';
  config.devServer = {
+    host: DEV_SERVER_HOST,
    port: DEV_SERVER_PORT,
    headers: { 'Access-Control-Allow-Origin': '*' },
    stats: 'errors-only',
    inline: DEV_SERVER_LIVERELOAD
  };
-  config.output.publicPath = '//localhost:' + DEV_SERVER_PORT + config.output.publicPath;
+  config.output.publicPath = '//' + DEV_SERVER_HOST + ':' + DEV_SERVER_PORT + config.output.publicPath;
  config.plugins.push(
    // watch node_modules for changes if we encounter a missing module compile error
    new WatchMissingNodeModulesPlugin(path.join(ROOT_PATH, 'node_modules'))
--- a/db/migrate/20130218141258_convert_closed_to_state_in_issue.rb
+++ b/db/migrate/20130218141258_convert_closed_to_state_in_issue.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class ConvertClosedToStateInIssue < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    execute "UPDATE #{table_name} SET state = 'closed' WHERE closed = #{true_value}"
--- a/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb
+++ b/db/migrate/20130218141327_convert_closed_to_state_in_merge_request.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class ConvertClosedToStateInMergeRequest < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    execute "UPDATE #{table_name} SET state = 'merged' WHERE closed = #{true_value} AND merged = #{true_value}"
--- a/db/migrate/20130218141344_convert_closed_to_state_in_milestone.rb
+++ b/db/migrate/20130218141344_convert_closed_to_state_in_milestone.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class ConvertClosedToStateInMilestone < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    execute "UPDATE #{table_name} SET state = 'closed' WHERE closed = #{true_value}"
--- a/db/migrate/20130315124931_user_color_scheme.rb
+++ b/db/migrate/20130315124931_user_color_scheme.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class UserColorScheme < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    add_column :users, :color_scheme_id, :integer, null: false, default: 1
--- a/db/migrate/20131112220935_add_visibility_level_to_projects.rb
+++ b/db/migrate/20131112220935_add_visibility_level_to_projects.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class AddVisibilityLevelToProjects < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def self.up
    add_column :projects, :visibility_level, :integer, :default => 0, :null => false
--- a/db/migrate/20140313092127_migrate_already_imported_projects.rb
+++ b/db/migrate/20140313092127_migrate_already_imported_projects.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class MigrateAlreadyImportedProjects < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    execute("UPDATE projects SET import_status = 'finished' WHERE imported = #{true_value}")
--- a/db/migrate/20141007100818_add_visibility_level_to_snippet.rb
+++ b/db/migrate/20141007100818_add_visibility_level_to_snippet.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class AddVisibilityLevelToSnippet < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    add_column :snippets, :visibility_level, :integer, :default => 0, :null => false
--- a/db/migrate/20151209144329_migrate_ci_web_hooks.rb
+++ b/db/migrate/20151209144329_migrate_ci_web_hooks.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class MigrateCiWebHooks < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    execute(
--- a/db/migrate/20151209145909_migrate_ci_emails.rb
+++ b/db/migrate/20151209145909_migrate_ci_emails.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class MigrateCiEmails < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    # This inserts a new service: BuildsEmailService
--- a/db/migrate/20151210125232_migrate_ci_slack_service.rb
+++ b/db/migrate/20151210125232_migrate_ci_slack_service.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class MigrateCiSlackService < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    properties_query = 'SELECT properties FROM ci_services ' \
--- a/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb
+++ b/db/migrate/20151210125927_migrate_ci_hip_chat_service.rb
@ -1,6 +1,6 @@
 # rubocop:disable all
 class MigrateCiHipChatService < ActiveRecord::Migration
-  include Gitlab::Database
+  include Gitlab::Database::MigrationHelpers

  def up
    # From properties strip `hipchat_` key
--- a/db/post_migrate/20170301205640_migrate_build_events_to_pipeline_events.rb
+++ b/db/post_migrate/20170301205640_migrate_build_events_to_pipeline_events.rb
@ -1,6 +1,5 @@
 class MigrateBuildEventsToPipelineEvents < ActiveRecord::Migration
  include Gitlab::Database::MigrationHelpers
-  include Gitlab::Database

  DOWNTIME = false

--- a/doc/ci/img/pipelines.png
+++ b/doc/ci/img/pipelines.png
--- a/doc/development/fe_guide/performance.md
+++ b/doc/development/fe_guide/performance.md
@ -48,8 +48,8 @@ Steps to split page-specific JavaScript from the main `main.js`:

 ```haml
 - content_for :page_specific_javascripts do
-  = page_specific_javascript_bundle_tag('lib_chart')
-  = page_specific_javascript_bundle_tag('graphs')
+  = webpack_bundle_tag 'lib_chart'
+  = webpack_bundle_tag 'graphs'
 ```

 The above loads `chart.js` and `graphs_bundle.js` for this page only. `chart.js`
--- a/doc/development/fe_guide/testing.md
+++ b/doc/development/fe_guide/testing.md
@ -13,10 +13,19 @@ for more information on general testing practices at GitLab.
 ## Karma test suite

 GitLab uses the [Karma][karma] test runner with [Jasmine][jasmine] as its test
-framework for our JavaScript unit tests.  For tests that rely on DOM
+framework for our JavaScript unit tests. For tests that rely on DOM
 manipulation we use fixtures which are pre-compiled from HAML source files and
 served during testing by the [jasmine-jquery][jasmine-jquery] plugin.

+JavaScript tests live in `spec/javascripts/`, matching the folder structure
+of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js`
+has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file.
+
+Keep in mind that in a CI environment, these tests are run in a headless
+browser and you will not have access to certain APIs, such as
+[`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification),
+which will have to be stubbed.
+
 ### Running frontend tests

 `rake karma` runs the frontend-only (JavaScript) tests.
@ -80,24 +89,23 @@ If an integration test depends on JavaScript to run correctly, you need to make
 sure the spec is configured to enable JavaScript when the tests are run. If you
 don't do this you'll see vague error messages from the spec runner.

-To enable a JavaScript driver in an `rspec` test, add `js: true` to the
+To enable a JavaScript driver in an `rspec` test, add `:js` to the
 individual spec or the context block containing multiple specs that need
 JavaScript enabled:

 ```ruby
-
 # For one spec
-it 'presents information about abuse report', js: true do
-    # assertions...
+it 'presents information about abuse report', :js do
+  # assertions...
 end

-describe "Admin::AbuseReports", js: true do
-    it 'presents information about abuse report' do
-        # assertions...
-    end
-    it 'shows buttons for adding to abuse report' do
-        # assertions...
-    end
+describe "Admin::AbuseReports", :js do
+  it 'presents information about abuse report' do
+    # assertions...
+  end
+  it 'shows buttons for adding to abuse report' do
+    # assertions...
+  end
 end
 ```

@ -113,13 +121,12 @@ file for the failing spec, add the `@javascript` flag above the Scenario:
 ```
@javascript
 Scenario: Developer can approve merge request
-    Given I am a "Shop" developer
-    And I visit project "Shop" merge requests page
-    And merge request 'Bug NS-04' must be approved
-    And I click link "Bug NS-04"
-    When I click link "Approve"
-    Then I should see approved merge request "Bug NS-04"
-
+  Given I am a "Shop" developer
+  And I visit project "Shop" merge requests page
+  And merge request 'Bug NS-04' must be approved
+  And I click link "Bug NS-04"
+  When I click link "Approve"
+  Then I should see approved merge request "Bug NS-04"
 ```

 [capybara]: http://teamcapybara.github.io/capybara/
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@ -4,28 +4,53 @@ When writing migrations for GitLab, you have to take into account that
 these will be ran by hundreds of thousands of organizations of all sizes, some with
 many years of data in their database.

-In addition, having to take a server offline for a an upgrade small or big is
-a big burden for most organizations. For this reason it is important that your
-migrations are written carefully, can be applied online and adhere to the style guide below.
+In addition, having to take a server offline for a a upgrade small or big is a
+big burden for most organizations. For this reason it is important that your
+migrations are written carefully, can be applied online and adhere to the style
+guide below.

-Migrations should not require GitLab installations to be taken offline unless
-_absolutely_ necessary - see the ["What Requires Downtime?"](what_requires_downtime.md)
-page. If a migration requires downtime, this should be clearly mentioned during
-the review process, as well as being documented in the monthly release post. For
-more information, see the "Downtime Tagging" section below.
+Migrations are **not** allowed to require GitLab installations to be taken
+offline unless _absolutely necessary_. Downtime assumptions should be based on
+the behaviour of a migration when performed using PostgreSQL, as various
+operations in MySQL may require downtime without there being alternatives.
+
+When downtime is necessary the migration has to be approved by:
+
+1. The VP of Engineering
+1. A Backend Lead
+1. A Database Specialist
+
+An up-to-date list of people holding these titles can be found at
+<https://about.gitlab.com/team/>.
+
+The document ["What Requires Downtime?"](what_requires_downtime.md) specifies
+various database operations, whether they require downtime and how to
+work around that whenever possible.

 When writing your migrations, also consider that databases might have stale data
-or inconsistencies and guard for that. Try to make as little assumptions as possible
-about the state of the database.
+or inconsistencies and guard for that. Try to make as few assumptions as
+possible about the state of the database.

-Please don't depend on GitLab specific code since it can change in future versions.
-If needed copy-paste GitLab code into the migration to make it forward compatible.
+Please don't depend on GitLab-specific code since it can change in future
+versions. If needed copy-paste GitLab code into the migration to make it forward
+compatible.
+
+## Commit Guidelines
+
+Each migration **must** be added in its own commit with a descriptive commit
+message. If a commit adds a migration it _should only_ include the migration and
+any corresponding changes to `db/schema.rb`. This makes it easy to revert a
+database migration without accidentally reverting other changes.

 ## Downtime Tagging

 Every migration must specify if it requires downtime or not, and if it should
-require downtime it must also specify a reason for this. To do so, add the
-following two constants to the migration class' body:
+require downtime it must also specify a reason for this. This is required even
+if 99% of the migrations won't require downtime as this makes it easier to find
+the migrations that _do_ require downtime.
+
+To tag a migration, add the following two constants to the migration class'
+body:

 * `DOWNTIME`: a boolean that when set to `true` indicates the migration requires
  downtime.
@ -50,12 +75,53 @@ from a migration class.

 ## Reversibility

-Your migration should be reversible. This is very important, as it should
+Your migration **must be** reversible. This is very important, as it should
 be possible to downgrade in case of a vulnerability or bugs.

 In your migration, add a comment describing how the reversibility of the
 migration was tested.

+## Multi Threading
+
+Sometimes a migration might need to use multiple Ruby threads to speed up a
+migration. For this to work your migration needs to include the module
+`Gitlab::Database::MultiThreadedMigration`:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  include Gitlab::Database::MultiThreadedMigration
+end
+```
+
+You can then use the method `with_multiple_threads` to perform work in separate
+threads. For example:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  include Gitlab::Database::MultiThreadedMigration
+
+  def up
+    with_multiple_threads(4) do
+      disable_statement_timeout
+
+      # ...
+    end
+  end
+end
+```
+
+Here the call to `disable_statement_timeout` will use the connection local to
+the `with_multiple_threads` block, instead of re-using the global connection
+pool.  This ensures each thread has its own connection object, and won't time
+out when trying to obtain one.
+
+**NOTE:** PostgreSQL has a maximum amount of connections that it allows. This
+limit can vary from installation to installation. As a result it's recommended
+you do not use more than 32 threads in a single migration. Usually 4-8 threads
+should be more than enough.
+
 ## Removing indices

 When removing an index make sure to use the method `remove_concurrent_index` instead
@ -78,7 +144,10 @@ end

 ## Adding indices

-If you need to add an unique index please keep in mind there is possibility of existing duplicates. If it is possible write a separate migration for handling this situation. It can be just removing or removing with overwriting all references to these duplicates depend on situation.
+If you need to add a unique index please keep in mind there is the possibility
+of existing duplicates being present in the database. This means that should
+always _first_ add a migration that removes any duplicates, before adding the
+unique index.

 When adding an index make sure to use the method `add_concurrent_index` instead
 of the regular `add_index` method. The `add_concurrent_index` method
@ -90,17 +159,22 @@ so:
 ```ruby
 class MyMigration < ActiveRecord::Migration
  include Gitlab::Database::MigrationHelpers
+
  disable_ddl_transaction!

-  def change
+  def up
+    add_concurrent_index :table, :column
+  end

+  def down
+    remove_index :table, :column if index_exists?(:table, :column)
  end
 end
 ```

 ## Adding Columns With Default Values

-When adding columns with default values you should use the method
+When adding columns with default values you must use the method
 `add_column_with_default`. This method ensures the table is updated without
 requiring downtime. This method is not reversible so you must manually define
 the `up` and `down` methods in your migration class.
@ -123,6 +197,9 @@ class MyMigration < ActiveRecord::Migration
 end
 ```

+Keep in mind that this operation can easily take 10-15 minutes to complete on
+larger installations (e.g. GitLab.com). As a result you should only add default
+values if absolutely necessary.

 ## Integer column type

@ -147,13 +224,15 @@ add_column(:projects, :foo, :integer, default: 10, limit: 8)

 ## Testing

-Make sure that your migration works with MySQL and PostgreSQL with data. An empty database does not guarantee that your migration is correct.
+Make sure that your migration works with MySQL and PostgreSQL with data. An
+empty database does not guarantee that your migration is correct.

 Make sure your migration can be reversed.

 ## Data migration

-Please prefer Arel and plain SQL over usual ActiveRecord syntax. In case of using plain SQL you need to quote all input manually with `quote_string` helper.
+Please prefer Arel and plain SQL over usual ActiveRecord syntax. In case of
+using plain SQL you need to quote all input manually with `quote_string` helper.

 Example with Arel:

@ -177,3 +256,17 @@ select_all("SELECT name, COUNT(id) as cnt FROM tags GROUP BY name HAVING COUNT(i
  execute("DELETE FROM tags WHERE id IN(#{duplicate_ids.join(",")})")
 end
 ```
+
+If you need more complex logic you can define and use models local to a
+migration. For example:
+
+```ruby
+class MyMigration < ActiveRecord::Migration
+  class Project < ActiveRecord::Base
+    self.table_name = 'projects'
+  end
+end
+```
+
+When doing so be sure to explicitly set the model's table name so it's not
+derived from the class name or namespace.
--- a/doc/development/testing.md
+++ b/doc/development/testing.md
@ -9,52 +9,179 @@ this guide defines a rule that contradicts the thoughtbot guide, this guide
 takes precedence. Some guidelines may be repeated verbatim to stress their
 importance.

-## Factories
+## Definitions

-GitLab uses [factory_girl] as a test fixture replacement.
+### Unit tests

- Factory definitions live in `spec/factories/`, named using the pluralization
-  of their corresponding model (`User` factories are defined in `users.rb`).
- There should be only one top-level factory definition per file.
- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and
-  should) call `create(...)` instead of `FactoryGirl.create(...)`.
- Make use of [traits] to clean up definitions and usages.
- When defining a factory, don't define attributes that are not required for the
-  resulting record to pass validation.
- When instantiating from a factory, don't supply attributes that aren't
-  required by the test.
- Factories don't have to be limited to `ActiveRecord` objects.
-  [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d).
+Formal definition: https://en.wikipedia.org/wiki/Unit_testing

-[factory_girl]: https://github.com/thoughtbot/factory_girl
-[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits
+These kind of tests ensure that a single unit of code (a method) works as
+expected (given an input, it has a predictable output). These tests should be
+isolated as much as possible. For example, model methods that don't do anything
+with the database shouldn't need a DB record. Classes that don't need database
+records should use stubs/doubles as much as possible.

-## JavaScript
+| Code path | Tests path | Testing engine | Notes |
+| --------- | ---------- | -------------- | ----- |
+| `app/finders/` | `spec/finders/` | RSpec | |
+| `app/helpers/` | `spec/helpers/` | RSpec | |
+| `app/db/{post_,}migrate/` | `spec/migrations/` | RSpec | |
+| `app/policies/` | `spec/policies/` | RSpec | |
+| `app/presenters/` | `spec/presenters/` | RSpec | |
+| `app/routing/` | `spec/routing/` | RSpec | |
+| `app/serializers/` | `spec/serializers/` | RSpec | |
+| `app/services/` | `spec/services/` | RSpec | |
+| `app/tasks/` | `spec/tasks/` | RSpec | |
+| `app/uploaders/` | `spec/uploaders/` | RSpec | |
+| `app/views/` | `spec/views/` | RSpec | |
+| `app/workers/` | `spec/workers/` | RSpec | |
+| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. |

-GitLab uses [Karma] to run its [Jasmine] JavaScript specs. They can be run on
-the command line via `bundle exec karma`.
+### Integration tests

- JavaScript tests live in `spec/javascripts/`, matching the folder structure
-  of `app/assets/javascripts/`: `app/assets/javascripts/behaviors/autosize.js`
-  has a corresponding `spec/javascripts/behaviors/autosize_spec.js` file.
- Haml fixtures required for JavaScript tests live in
-  `spec/javascripts/fixtures`. They should contain the bare minimum amount of
-  markup necessary for the test.
+Formal definition: https://en.wikipedia.org/wiki/Integration_testing

-    > **Warning:** Keep in mind that a Rails view may change and
-    invalidate your test, but everything will still pass because your fixture
-    doesn't reflect the latest view. Because of this we encourage you to
-    generate fixtures from actual rails views whenever possible.
+These kind of tests ensure that individual parts of the application work well together, without the overhead of the actual app environment (i.e. the browser). These tests should assert at the request/response level: status code, headers, body. They're useful to test permissions, redirections, what view is rendered etc.

- Keep in mind that in a CI environment, these tests are run in a headless
-  browser and you will not have access to certain APIs, such as
-  [`Notification`](https://developer.mozilla.org/en-US/docs/Web/API/notification),
-  which will have to be stubbed.
+| Code path | Tests path | Testing engine | Notes |
+| --------- | ---------- | -------------- | ----- |
+| `app/controllers/` | `spec/controllers/` | RSpec | |
+| `app/mailers/` | `spec/mailers/` | RSpec | |
+| `lib/api/` | `spec/requests/api/` | RSpec | |
+| `lib/ci/api/` | `spec/requests/ci/api/` | RSpec | |
+| `app/assets/javascripts/` | `spec/javascripts/` | Karma | More details in the [JavaScript](#javascript) section. |

-[Karma]: https://github.com/karma-runner/karma
-[Jasmine]: https://github.com/jasmine/jasmine
+#### About controller tests

-For more information, see the [frontend testing guide](fe_guide/testing.md).
+In an ideal world, controllers should be thin. However, when this is not the
+case, it's acceptable to write a system/feature test without JavaScript instead
+of a controller test. The reason is that testing a fat controller usually
+involves a lot of stubbing, things like:
+
+```ruby
+controller.instance_variable_set(:@user, user)
+```
+
+and use methods which are deprecated in Rails 5 ([#23768]).
+
+[#23768]: https://gitlab.com/gitlab-org/gitlab-ce/issues/23768
+
+#### About Karma
+
+As you may have noticed, Karma is both in the Unit tests and the Integration
+tests category. That's because Karma is a tool that provides an environment to
+run JavaScript tests, so you can either run unit tests (e.g. test a single
+JavaScript method), or integration tests (e.g. test a component that is composed
+of multiple components).
+
+### System tests or Feature tests
+
+Formal definition: https://en.wikipedia.org/wiki/System_testing.
+
+These kind of tests ensure the application works as expected from a user point
+of view (aka black-box testing). These tests should test a happy path for a
+given page or set of pages, and a test case should be added for any regression
+that couldn't have been caught at lower levels with better tests (i.e. if a
+regression is found, regression tests should be added at the lowest-level
+possible).
+
+| Tests path | Testing engine | Notes |
+| ---------- | -------------- | ----- |
+| `spec/features/` | [Capybara] + [RSpec] | If your spec has the `:js` metadata, the browser driver will be [Poltergeist], otherwise it's using [RackTest]. |
+| `features/` | Spinach | Spinach tests are deprecated, [you shouldn't add new Spinach tests](#spinach-feature-tests). |
+
+[Capybara]: https://github.com/teamcapybara/capybara
+[RSpec]: https://github.com/rspec/rspec-rails#feature-specs
+[Poltergeist]: https://github.com/teamcapybara/capybara#poltergeist
+[RackTest]: https://github.com/teamcapybara/capybara#racktest
+
+#### Best practices
+
+- Create only the necessary records in the database
+- Test a happy path and a less happy path but that's it
+- Every other possible path should be tested with Unit or Integration tests
+- Test what's displayed on the page, not the internals of ActiveRecord models.
+  For instance, if you want to verify that a record was created, add
+  expectations that its attributes are displayed on the page, not that
+  `Model.count` increased by one.
+- It's ok to look for DOM elements but don't abuse it since it makes the tests
+  more brittle
+
+If we're confident that the low-level components work well (and we should be if
+we have enough Unit & Integration tests), we shouldn't need to duplicate their
+thorough testing at the System test level.
+
+It's very easy to add tests, but a lot harder to remove or improve tests, so one
+should take care of not introducing too many (slow and duplicated) specs.
+
+The reasons why we should follow these best practices are as follows:
+
+- System tests are slow to run since they spin up the entire application stack
+  in a headless browser, and even slower when they integrate a JS driver
+- When system tests run with a JavaScript driver, the tests are run in a
+  different thread than the application. This means it does not share a
+  database connection and your test will have to commit the transactions in
+  order for the running application to see the data (and vice-versa). In that
+  case we need to truncate the database after each spec instead of simply
+  rolling back a transaction (the faster strategy that's in use for other kind
+  of tests). This is slower than transactions, however, so we want to use
+  truncation only when necessary.
+
+### Black-box tests or End-to-end tests
+
+GitLab consists of [multiple pieces] such as [GitLab Shell], [GitLab Workhorse],
+[Gitaly], [GitLab Pages], [GitLab Runner], and GitLab Rails. All theses pieces
+are configured and packaged by [GitLab Omnibus].
+
+[GitLab QA] is a tool that allows to test that all these pieces integrate well
+together by building a Docker image for a given version of GitLab Rails and
+running feature tests (i.e. using Capybara) against it.
+
+The actual test scenarios and steps are [part of GitLab Rails] so that they're
+always in-sync with the codebase.
+
+[multiple pieces]: ./architecture.md#components
+[GitLab Shell]: https://gitlab.com/gitlab-org/gitlab-shell
+[GitLab Workhorse]: https://gitlab.com/gitlab-org/gitlab-workhorse
+[Gitaly]: https://gitlab.com/gitlab-org/gitaly
+[GitLab Pages]: https://gitlab.com/gitlab-org/gitlab-pages
+[GitLab Runner]: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner
+[GitLab Omnibus]: https://gitlab.com/gitlab-org/omnibus-gitlab
+[GitLab QA]: https://gitlab.com/gitlab-org/gitlab-qa
+[part of GitLab Rails]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa
+
+## How to test at the correct level?
+
+As many things in life, deciding what to test at each level of testing is a
+trade-off:
+
+- Unit tests are usually cheap, and you should consider them like the basement
+  of your house: you need them to be confident that your code is behaving
+  correctly. However if you run only unit tests without integration / system tests, you might [miss] the [big] [picture]!
+- Integration tests are a bit more expensive, but don't abuse them. A feature test
+  is often better than an integration test that is stubbing a lot of internals.
+- System tests are expensive (compared to unit tests), even more if they require
+  a JavaScript driver. Make sure to follow the guidelines in the [Speed](#test-speed)
+  section.
+
+Another way to see it is to think about the "cost of tests", this is well
+explained [in this article][tests-cost] and the basic idea is that the cost of a
+test includes:
+
+- The time it takes to write the test
+- The time it takes to run the test every time the suite runs
+- The time it takes to understand the test
+- The time it takes to fix the test if it breaks and the underlying code is OK
+- Maybe, the time it takes to change the code to make the code testable.
+
+[miss]: https://twitter.com/ThePracticalDev/status/850748070698651649
+[big]: https://twitter.com/timbray/status/822470746773409794
+[picture]: https://twitter.com/withzombies/status/829716565834752000
+[tests-cost]: https://medium.com/table-xi/high-cost-tests-and-high-value-tests-a86e27a54df#.2ulyh3a4e
+
+## Frontend testing
+
+Please consult the [dedicated "Frontend testing" guide](./fe_guide/testing.md).

 ## RSpec

@ -117,53 +244,124 @@ it 'is overdue' do
 end
 ```

-### Test speed
+### System / Feature tests

-GitLab has a massive test suite that, without parallelization, can take more
-than an hour to run. It's important that we make an effort to write tests that
-are accurate and effective _as well as_ fast.
-
-Here are some things to keep in mind regarding test performance:
-
- `double` and `spy` are faster than `FactoryGirl.build(...)`
- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`.
- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`,
-  `spy`, or `double` will do. Database persistence is slow!
- Use `create(:empty_project)` instead of `create(:project)` when you don't need
-  the underlying Git repository. Filesystem operations are slow!
- Don't mark a feature as requiring JavaScript (through `@javascript` in
-  Spinach or `js: true` in RSpec) unless it's _actually_ required for the test
-  to be valid. Headless browser testing is slow!
-
-### Features / Integration
-
-GitLab uses [rspec-rails feature specs] to test features in a browser
-environment. These are [capybara] specs running on the headless [poltergeist]
-driver.
-
- Feature specs live in `spec/features/` and should be named
-  `ROLE_ACTION_spec.rb`, such as `user_changes_password_spec.rb`.
+- Feature specs should be named `ROLE_ACTION_spec.rb`, such as
+  `user_changes_password_spec.rb`.
 - Use only one `feature` block per feature spec file.
 - Use scenario titles that describe the success and failure paths.
- Avoid scenario titles that add no information, such as "successfully."
+- Avoid scenario titles that add no information, such as "successfully".
 - Avoid scenario titles that repeat the feature title.

-[rspec-rails feature specs]: https://github.com/rspec/rspec-rails#feature-specs
-[capybara]: https://github.com/teamcapybara/capybara
-[poltergeist]: https://github.com/teampoltergeist/poltergeist
+### Matchers

-## Spinach (feature) tests
+Custom matchers should be created to clarify the intent and/or hide the
+complexity of RSpec expectations.They should be placed under
+`spec/support/matchers/`. Matchers can be placed in subfolder if they apply to
+a certain type of specs only (e.g. features, requests etc.) but shouldn't be if
+they apply to multiple type of specs.

-GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426)
-for its feature/integration tests in September 2012.
+### Shared contexts

-As of March 2016, we are [trying to avoid adding new Spinach
-tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward,
-opting for [RSpec feature](#features-integration) specs.
+All shared contexts should be be placed under `spec/support/shared_contexts/`.
+Shared contexts can be placed in subfolder if they apply to a certain type of
+specs only (e.g. features, requests etc.) but shouldn't be if they apply to
+multiple type of specs.

-Adding new Spinach scenarios is acceptable _only if_ the new scenario requires
-no more than one new `step` definition. If more than that is required, the
-test should be re-implemented using RSpec instead.
+Each file should include only one context and have a descriptive name, e.g.
+`spec/support/shared_contexts/controllers/githubish_import_controller_shared_context.rb`.
+
+### Shared examples
+
+All shared examples should be be placed under `spec/support/shared_examples/`.
+Shared examples can be placed in subfolder if they apply to a certain type of
+specs only (e.g. features, requests etc.) but shouldn't be if they apply to
+multiple type of specs.
+
+Each file should include only one context and have a descriptive name, e.g.
+`spec/support/shared_examples/controllers/githubish_import_controller_shared_example.rb`.
+
+### Helpers
+
+Helpers are usually modules that provide some methods to hide the complexity of
+specific RSpec examples. You can define helpers in RSpec files if they're not
+intended to be shared with other specs. Otherwise, they should be be placed
+under `spec/support/helpers/`. Helpers can be placed in subfolder if they apply
+to a certain type of specs only (e.g. features, requests etc.) but shouldn't be
+if they apply to multiple type of specs.
+
+Helpers should follow the Rails naming / namespacing convention. For instance
+`spec/support/helpers/cycle_analytics_helpers.rb` should define:
+
+```ruby
+module Spec
+  module Support
+    module Helpers
+      module CycleAnalyticsHelpers
+        def create_commit_referencing_issue(issue, branch_name: random_git_name)
+          project.repository.add_branch(user, branch_name, 'master')
+          create_commit("Commit for ##{issue.iid}", issue.project, user, branch_name)
+        end
+      end
+    end
+  end
+end
+```
+
+Helpers should not change the RSpec config. For instance, the helpers module
+described above should not include:
+
+```ruby
+RSpec.configure do |config|
+  config.include Spec::Support::Helpers::CycleAnalyticsHelpers
+end
+```
+
+### Factories
+
+GitLab uses [factory_girl] as a test fixture replacement.
+
+- Factory definitions live in `spec/factories/`, named using the pluralization
+  of their corresponding model (`User` factories are defined in `users.rb`).
+- There should be only one top-level factory definition per file.
+- FactoryGirl methods are mixed in to all RSpec groups. This means you can (and
+  should) call `create(...)` instead of `FactoryGirl.create(...)`.
+- Make use of [traits] to clean up definitions and usages.
+- When defining a factory, don't define attributes that are not required for the
+  resulting record to pass validation.
+- When instantiating from a factory, don't supply attributes that aren't
+  required by the test.
+- Factories don't have to be limited to `ActiveRecord` objects.
+  [See example](https://gitlab.com/gitlab-org/gitlab-ce/commit/0b8cefd3b2385a21cfed779bd659978c0402766d).
+
+[factory_girl]: https://github.com/thoughtbot/factory_girl
+[traits]: http://www.rubydoc.info/gems/factory_girl/file/GETTING_STARTED.md#Traits
+
+### Fixtures
+
+All fixtures should be be placed under `spec/fixtures/`.
+
+### Config
+
+RSpec config files are files that change the RSpec config (i.e.
+`RSpec.configure do |config|` blocks). They should be placed under
+`spec/support/config/`.
+
+Each file should be related to a specific domain, e.g.
+`spec/support/config/capybara.rb`, `spec/support/config/carrierwave.rb`, etc.
+
+Helpers can be included in the `spec/support/config/rspec.rb` file. If a
+helpers module applies only to a certain kind of specs, it should add modifiers
+to the `config.include` call. For instance if
+`spec/support/helpers/cycle_analytics_helpers.rb` applies to `:lib` and
+`type: :model` specs only, you would write the following:
+
+```ruby
+RSpec.configure do |config|
+  config.include Spec::Support::Helpers::CycleAnalyticsHelpers, :lib
+  config.include Spec::Support::Helpers::CycleAnalyticsHelpers, type: :model
+end
+```

 ## Testing Rake Tasks

@ -201,6 +399,77 @@ describe 'gitlab:shell rake tasks' do
 end
 ```

+## Test speed
+
+GitLab has a massive test suite that, without [parallelization], can take hours
+to run. It's important that we make an effort to write tests that are accurate
+and effective _as well as_ fast.
+
+Here are some things to keep in mind regarding test performance:
+
+- `double` and `spy` are faster than `FactoryGirl.build(...)`
+- `FactoryGirl.build(...)` and `.build_stubbed` are faster than `.create`.
+- Don't `create` an object when `build`, `build_stubbed`, `attributes_for`,
+  `spy`, or `double` will do. Database persistence is slow!
+- Use `create(:empty_project)` instead of `create(:project)` when you don't need
+  the underlying Git repository. Filesystem operations are slow!
+- Don't mark a feature as requiring JavaScript (through `@javascript` in
+  Spinach or `:js` in RSpec) unless it's _actually_ required for the test
+  to be valid. Headless browser testing is slow!
+
+[parallelization]: #test-suite-parallelization-on-the-ci
+
+### Test suite parallelization on the CI
+
+Our current CI parallelization setup is as follows:
+
+1. The `knapsack` job in the prepare stage that is supposed to ensure we have a
+  `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file:
+  - The `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file is fetched
+    from S3, if it's not here we initialize the file with `{}`.
+1. Each `rspec x y` job are run with `knapsack rspec` and should have an evenly
+  distributed share of tests:
+  - It works because the jobs have access to the
+    `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` since the "artifacts
+    from all previous stages are passed by default". [^1]
+  - the jobs set their own report path to
+    `KNAPSACK_REPORT_PATH=knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`.
+  - if knapsack is doing its job, test files that are run should be listed under
+    `Report specs`, not under `Leftover specs`.
+1. The `update-knapsack` job takes all the
+  `knapsack/${CI_PROJECT_NAME}/${JOB_NAME[0]}_node_${CI_NODE_INDEX}_${CI_NODE_TOTAL}_report.json`
+  files from the `rspec x y` jobs and merge them all together into a single
+  `knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file that is then
+  uploaded to S3.
+
+After that, the next pipeline will use the up-to-date
+`knapsack/${CI_PROJECT_NAME}/rspec_report-master.json` file. The same strategy
+is used for Spinach tests as well.
+
+### Monitoring
+
+The GitLab test suite is [monitored] and a [public dashboard] is available for
+everyone to see. Feel free to look at the slowest test files and try to improve
+them.
+
+[monitored]: ./performance.md#rspec-profiling
+[public dashboard]: https://redash.gitlab.com/public/dashboards/l1WhHXaxrCWM5Ai9D7YDqHKehq6OU3bx5gssaiWe?org_slug=default
+
+## Spinach (feature) tests
+
+GitLab [moved from Cucumber to Spinach](https://github.com/gitlabhq/gitlabhq/pull/1426)
+for its feature/integration tests in September 2012.
+
+As of March 2016, we are [trying to avoid adding new Spinach
+tests](https://gitlab.com/gitlab-org/gitlab-ce/issues/14121) going forward,
+opting for [RSpec feature](#features-integration) specs.
+
+Adding new Spinach scenarios is acceptable _only if_ the new scenario requires
+no more than one new `step` definition. If more than that is required, the
+test should be re-implemented using RSpec instead.
+
 ---

 [Return to Development documentation](README.md)
+
+[^1]: /ci/yaml/README.html#dependencies
--- a/doc/development/what_requires_downtime.md
+++ b/doc/development/what_requires_downtime.md
@ -2,7 +2,8 @@

 When working with a database certain operations can be performed without taking
 GitLab offline, others do require a downtime period. This guide describes
-various operations and their impact.
+various operations, their impact, and how to perform them without requiring
+downtime.

 ## Adding Columns

@ -41,50 +42,156 @@ information on how to use this method.

 ## Dropping Columns

-On PostgreSQL you can safely remove an existing column without the need for
-downtime. When you drop a column in PostgreSQL it's not immediately removed,
-instead it is simply disabled. The data is removed on the next vacuum run.
+Removing columns is tricky because running GitLab processes may still be using
+the columns. To work around this you will need two separate merge requests and
+releases: one to ignore and then remove the column, and one to remove the ignore
+rule.

-On MySQL this operation requires downtime.
+### Step 1: Ignoring The Column

-While database wise dropping a column may be fine on PostgreSQL this operation
-still requires downtime because the application code may still be using the
-column that was removed. For example, consider the following migration:
+The first step is to ignore the column in the application code. This is
+necessary because Rails caches the columns and re-uses this cache in various
+places. This can be done by including the `IgnorableColumn` module into the
+model, followed by defining the columns to ignore. For example, to ignore
+`updated_at` in the User model you'd use the following:

 ```ruby
-class MyMigration < ActiveRecord::Migration
-  def change
-    remove_column :projects, :dummy
+class User < ActiveRecord::Base
+  include IgnorableColumn
+
+  ignore_column :updated_at
+end
+```
+
+Once added you should create a _post-deployment_ migration that removes the
+column. Both these changes should be submitted in the same merge request.
+
+### Step 2: Removing The Ignore Rule
+
+Once the changes from step 1 have been released & deployed you can set up a
+separate merge request that removes the ignore rule. This merge request can
+simply remove the `ignore_column` line, and the `include IgnorableColumn` line
+if no other `ignore_column` calls remain.
+
+## Renaming Columns
+
+Renaming columns the normal way requires downtime as an application may continue
+using the old column name during/after a database migration. To rename a column
+without requiring downtime we need two migrations: a regular migration, and a
+post-deployment migration. Both these migration can go in the same release.
+
+### Step 1: Add The Regular Migration
+
+First we need to create the regular migration. This migration should use
+`Gitlab::Database::MigrationHelpers#rename_column_concurrently` to perform the
+renaming. For example
+
+```ruby
+# A regular migration in db/migrate
+class RenameUsersUpdatedAtToUpdatedAtTimestamp < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    rename_column_concurrently :users, :updated_at, :updated_at_timestamp
+  end
+
+  def down
+    cleanup_concurrent_column_rename :users, :updated_at_timestamp, :updated_at
  end
 end
 ```

-Now imagine that the GitLab instance is running and actively uses the `dummy`
-column. If we were to run the migration this would result in the GitLab instance
-producing errors whenever it tries to use the `dummy` column.
+This will take care of renaming the column, ensuring data stays in sync, copying
+over indexes and foreign keys, etc.

-As a result of the above downtime _is_ required when removing a column, even
-when using PostgreSQL.
+**NOTE:** if a column contains 1 or more indexes that do not contain the name of
+the original column, the above procedure will fail. In this case you will first
+need to rename these indexes.

-## Renaming Columns
+### Step 2: Add A Post-Deployment Migration

-Renaming columns requires downtime as running GitLab instances will continue
-using the old column name until a new version is deployed. This can result
-in the instance producing errors, which in turn can impact the user experience.
+The renaming procedure requires some cleaning up in a post-deployment migration.
+We can perform this cleanup using
+`Gitlab::Database::MigrationHelpers#cleanup_concurrent_column_rename`:
+
+```ruby
+# A post-deployment migration in db/post_migrate
+class CleanupUsersUpdatedAtRename < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_rename :users, :updated_at, :updated_at_timestamp
+  end
+
+  def down
+    rename_column_concurrently :users, :updated_at_timestamp, :updated_at
+  end
+end
+```

 ## Changing Column Constraints

-Generally changing column constraints requires checking all rows in the table to
-see if they meet the new constraint, unless a constraint is _removed_. For
-example, changing a column that previously allowed NULL values to not allow NULL
-values requires the database to verify all existing rows.
-
-The specific behaviour varies a bit between databases but in general the safest
-approach is to assume changing constraints requires downtime.
+Adding or removing a NOT NULL clause (or another constraint) can typically be
+done without requiring downtime. However, this does require that any application
+changes are deployed _first_. Thus, changing the constraints of a column should
+happen in a post-deployment migration.

 ## Changing Column Types

-This operation requires downtime.
+Changing the type of a column can be done using
+`Gitlab::Database::MigrationHelpers#change_column_type_concurrently`. This
+method works similarly to `rename_column_concurrently`. For example, let's say
+we want to change the type of `users.username` from `string` to `text`.
+
+### Step 1: Create A Regular Migration
+
+A regular migration is used to create a new column with a temporary name along
+with setting up some triggers to keep data in sync. Such a migration would look
+as follows:
+
+```ruby
+# A regular migration in db/migrate
+class ChangeUsersUsernameStringToText < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    change_column_type_concurrently :users, :username, :text
+  end
+
+  def down
+    cleanup_concurrent_column_type_change :users, :username
+  end
+end
+```
+
+### Step 2: Create A Post Deployment Migration
+
+Next we need to clean up our changes using a post-deployment migration:
+
+```ruby
+# A post-deployment migration in db/post_migrate
+class ChangeUsersUsernameStringToTextCleanup < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  def up
+    cleanup_concurrent_column_type_change :users
+  end
+
+  def down
+    change_column_type_concurrently :users, :username, :string
+  end
+end
+```
+
+And that's it, we're done!

 ## Adding Indexes

@ -101,12 +208,19 @@ Migrations can take advantage of this by using the method

 ```ruby
 class MyMigration < ActiveRecord::Migration
-  def change
+  def up
    add_concurrent_index :projects, :column_name
  end
+
+  def down
+    remove_index(:projects, :column_name) if index_exists?(:projects, :column_name)
+  end
 end
 ```

+Note that `add_concurrent_index` can not be reversed automatically, thus you
+need to manually define `up` and `down`.
+
 When running this on PostgreSQL the `CONCURRENTLY` option mentioned above is
 used. On MySQL this method produces a regular `CREATE INDEX` query.

@ -125,43 +239,54 @@ This operation is safe as there's no code using the table just yet.

 ## Dropping Tables

-This operation requires downtime as application code may still be using the
-table.
+Dropping tables can be done safely using a post-deployment migration, but only
+if the application no longer uses the table.

 ## Adding Foreign Keys

-Adding foreign keys acquires an exclusive lock on both the source and target
-tables in PostgreSQL. This requires downtime as otherwise the entire application
-grinds to a halt for the duration of the operation.
+Adding foreign keys usually works in 3 steps:

-On MySQL this operation also requires downtime _unless_ foreign key checks are
-disabled. Because this means checks aren't enforced this is not ideal, as such
-one should assume MySQL also requires downtime.
+1. Start a transaction
+1. Run `ALTER TABLE` to add the constraint(s)
+1. Check all existing data
+
+Because `ALTER TABLE` typically acquires an exclusive lock until the end of a
+transaction this means this approach would require downtime.
+
+GitLab allows you to work around this by using
+`Gitlab::Database::MigrationHelpers#add_concurrent_foreign_key`. This method
+ensures that when PostgreSQL is used no downtime is needed.

 ## Removing Foreign Keys

-This operation should not require downtime on both PostgreSQL and MySQL.
+This operation does not require downtime.

-## Updating Data
+## Data Migrations

-Updating data should generally be safe. The exception to this is data that's
-being migrated from one version to another while the application still produces
-data in the old version.
+Data migrations can be tricky. The usual approach to migrate data is to take a 3
+step approach:

-For example, imagine the application writes the string `'dog'` to a column but
-it really is meant to write `'cat'` instead. One might think that the following
-migration is all that is needed to solve this problem:
+1. Migrate the initial batch of data
+1. Deploy the application code
+1. Migrate any remaining data

-```ruby
-class MyMigration < ActiveRecord::Migration
-  def up
-    execute("UPDATE some_table SET column = 'cat' WHERE column = 'dog';")
-  end
-end
-```
+Usually this works, but not always. For example, if a field's format is to be
+changed from JSON to something else we have a bit of a problem. If we were to
+change existing data before deploying application code we'll most likely run
+into errors. On the other hand, if we were to migrate after deploying the
+application code we could run into the same problems.

-Unfortunately this is not enough. Because the application is still running and
-using the old value this may result in the table still containing rows where
-`column` is set to `dog`, even after the migration finished.
+If you merely need to correct some invalid data, then a post-deployment
+migration is usually enough. If you need to change the format of data (e.g. from
+JSON to something else) it's typically best to add a new column for the new data
+format, and have the application use that. In such a case the procedure would
+be:

-In these cases downtime _is_ required, even for rarely updated tables.
+1. Add a new column in the new format
+1. Copy over existing data to this new column
+1. Deploy the application code
+1. In a post-deployment migration, copy over any remaining data
+
+In general there is no one-size-fits-all solution, therefore it's best to
+discuss these kind of migrations in a merge request to make sure they are
+implemented in the best way possible.
--- a/doc/install/installation.md
+++ b/doc/install/installation.md
@ -289,9 +289,9 @@ sudo usermod -aG redis git
 ### Clone the Source

    # Clone GitLab repository
-    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-0-stable gitlab
+    sudo -u git -H git clone https://gitlab.com/gitlab-org/gitlab-ce.git -b 9-1-stable gitlab

-**Note:** You can change `9-0-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server!
+**Note:** You can change `9-1-stable` to `master` if you want the *bleeding edge* version, but never install master on a production server!

 ### Configure It

@ -475,7 +475,7 @@ with setting up Gitaly until you upgrade to GitLab 9.2 or later.
    sudo -u git cp config.toml.example config.toml
    # If you are using non-default settings you need to update config.toml
    sudo -u git -H editor config.toml
-    
+
    # Enable Gitaly in the init script
    echo 'gitaly_enabled=true' | sudo tee -a /etc/default/gitlab

--- a/doc/topics/authentication/index.md
+++ b/doc/topics/authentication/index.md
@ -0,0 +1,46 @@
+# Authentication
+
+This page gathers all the resources for the topic **Authentication** within GitLab.
+
+## GitLab users
+
+- [SSH](../../ssh/README.md)
+- [Two-Factor Authentication (2FA)](../../user/profile/account/two_factor_authentication.md#two-factor-authentication)
+- **Articles:**
+  - [Support for Universal 2nd Factor Authentication - YubiKeys](https://about.gitlab.com/2016/06/22/gitlab-adds-support-for-u2f/)
+  - [Security Webcast with Yubico](https://about.gitlab.com/2016/08/31/gitlab-and-yubico-security-webcast/)
+- **Integrations:**
+  - [GitLab as OAuth2 authentication service provider](../../integration/oauth_provider.md#introduction-to-oauth)
+
+## GitLab administrators
+
+- [LDAP (Community Edition)](../../administration/auth/ldap.md)
+- [LDAP (Enterprise Edition)](https://docs.gitlab.com/ee/administration/auth/ldap-ee.html)
+- [Enforce Two-factor Authentication (2FA)](../../security/two_factor_authentication.md#enforce-two-factor-authentication-2fa)
+- **Articles:**
+  - [Feature Highlight: LDAP Integration](https://about.gitlab.com/2014/07/10/feature-highlight-ldap-sync/)
+  - [Debugging LDAP](https://about.gitlab.com/handbook/support/workflows/ldap/debugging_ldap.html)
+- **Integrations:**
+  - [OmniAuth](../../integration/omniauth.md)
+  - [Authentiq OmniAuth Provider](../../administration/auth/authentiq.md#authentiq-omniauth-provider)
+  - [Atlassian Crowd OmniAuth Provider](../../administration/auth/crowd.md)
+  - [CAS OmniAuth Provider](../../integration/cas.md)
+  - [SAML OmniAuth Provider](../../integration/saml.md)
+  - [Okta SSO provider](../../administration/auth/okta.md)
+  - [Kerberos integration (GitLab EE)](https://docs.gitlab.com/ee/integration/kerberos.html)
+
+## API
+
+- [OAuth 2 Tokens](../../api/README.md#oauth-2-tokens)
+- [Private Tokens](../../api/README.md#private-tokens)
+- [Impersonation tokens](../../api/README.md#impersonation-tokens)
+- [GitLab as an OAuth2 provider](../../api/oauth2.md#gitlab-as-an-oauth2-provider)
+- [GitLab Runner API - Authentication](../../api/ci/runners.md#authentication)
+
+## Third-party resources
+
+- [Kanboard Plugin GitLab Authentication](https://kanboard.net/plugin/gitlab-auth)
+- [Jenkins GitLab OAuth Plugin](https://wiki.jenkins-ci.org/display/JENKINS/GitLab+OAuth+Plugin)
+- [Setup Gitlab CE with Active Directory authentication](https://www.caseylabs.com/setup-gitlab-ce-with-active-directory-authentication/)
+- [How to customize GitLab to support OpenID authentication](http://eric.van-der-vlist.com/blog/2013/11/23/how-to-customize-gitlab-to-support-openid-authentication/)
+- [Openshift - Configuring Authentication and User Agent](https://docs.openshift.org/latest/install_config/configuring_authentication.html#GitLab)
--- a/doc/topics/git/index.md
+++ b/doc/topics/git/index.md
@ -0,0 +1,61 @@
+# Git documentation
+
+Git is a [free and open source](https://git-scm.com/about/free-and-open-source)
+distributed version control system designed to handle everything from small to
+very large projects with speed and efficiency.
+
+[GitLab](https://about.gitlab.com) is a Git-based fully integrated platform for
+software development. Besides Git's functionalities, GitLab has a lot of
+powerful [features](https://about.gitlab.com/features/) to enhance your
+[workflow](https://about.gitlab.com/2016/10/25/gitlab-workflow-an-overview/).
+
+We've gathered some resources to help you to get the best from Git with GitLab.
+
+## Getting started
+
+- [Git concepts](../../university/training/user_training.md#git-concepts)
+- [Start using Git on the command line](../../gitlab-basics/start-using-git.md)
+- [Command Line basic commands](../../gitlab-basics/command-line-commands.md)
+- [GitLab Git Cheat Sheet (download)](https://gitlab.com/gitlab-com/marketing/raw/master/design/print/git-cheatsheet/print-pdf/git-cheatsheet.pdf)
+- **Articles:**
+  - [Git Tips & Tricks](https://about.gitlab.com/2016/12/08/git-tips-and-tricks/)
+  - [Eight Tips to help you work better with Git](https://about.gitlab.com/2015/02/19/8-tips-to-help-you-work-better-with-git/)
+- **Presentations:**
+  - [GLU Course: About Version Control](https://docs.google.com/presentation/d/16sX7hUrCZyOFbpvnrAFrg6tVO5_yT98IgdAqOmXwBho/edit?usp=sharing)
+- **Third-party resources:**
+  - What is [Git](https://git-scm.com)
+    - [Version control](https://git-scm.com/book/en/v2/Getting-Started-About-Version-Control)
+    - [Getting Started - Git Basics](https://git-scm.com/book/en/v2/Getting-Started-Git-Basics)
+    - [Getting Started - Installing Git](https://git-scm.com/book/en/v2/Getting-Started-Installing-Git)
+    - [Git on the Server - GitLab](https://git-scm.com/book/en/v2/Git-on-the-Server-GitLab)
+
+## Branching strategies
+
+- **Articles:**
+  - [GitLab Flow](https://about.gitlab.com/2014/09/29/gitlab-flow/)
+- **Third-party resources:**
+  - [Git Branching - Branches in a Nutshell](https://git-scm.com/book/en/v2/Git-Branching-Branches-in-a-Nutshell)
+  - [Git Branching - Branching Workflows](https://git-scm.com/book/en/v2/Git-Branching-Branching-Workflows)
+
+## Advanced use
+
+- [Custom Git Hooks](../../administration/custom_hooks.md)
+- [Git Attributes](../../user/project/git_attributes.md)
+- Git Submodules: [Using Git submodules with GitLab CI](../../ci/git_submodules.md#using-git-submodules-with-gitlab-ci)
+
+## API
+
+- [Gitignore templates](../../api/templates/gitignores.md)
+
+## Git LFS
+
+- [Git LFS](../../workflow/lfs/manage_large_binaries_with_git_lfs.md)
+- [Git-Annex to Git-LFS migration guide](https://docs.gitlab.com/ee/workflow/lfs/migrate_from_git_annex_to_git_lfs.html)
+- **Articles:**
+  - [Getting Started with Git LFS](https://about.gitlab.com/2017/01/30/getting-started-with-git-lfs-tutorial/)
+  - [Towards a production quality open source Git LFS server](https://about.gitlab.com/2015/08/13/towards-a-production-quality-open-source-git-lfs-server/)
+
+## General information
+
+- **Articles:**
+  - [The future of SaaS hosted Git repository pricing](https://about.gitlab.com/2016/05/11/git-repository-pricing/)
--- a/doc/topics/index.md
+++ b/doc/topics/index.md
@ -7,10 +7,10 @@ you through better understanding GitLab's concepts
 through our regular docs, and, when available, through articles (guides,
 tutorials, technical overviews, blog posts) and videos.

- [GitLab Installation](../install/README.md)
+- [Authentication](authentication/index.md)
 - [Continuous Integration (GitLab CI)](../ci/README.md)
+- [Git](git/index.md)
+- [GitLab Installation](../install/README.md)
 - [GitLab Pages](../user/project/pages/index.md)

->**Note:**
-Non-linked topics are currently under development and subjected to change.
-More topics will be available soon.
+>**Note:** More topics will be available soon.
--- a/doc/update/9.0-to-9.1.md
+++ b/doc/update/9.0-to-9.1.md
@ -1,9 +1,5 @@
 # From 9.0 to 9.1

-** TODO: **
-
-# TODO clean out 9.0-specific stuff
-
 Make sure you view this update guide from the tag (version) of GitLab you would
 like to install. In most cases this should be the highest numbered production
 tag (without rc in it). You can select the tag in the version dropdown at the
--- a/doc/update/README.md
+++ b/doc/update/README.md
@ -48,6 +48,23 @@ GitLab provides official Docker images for both Community and Enterprise
 editions. They are based on the Omnibus package and instructions on how to
 update them are in [a separate document][omnidocker].

+## Upgrading without downtime
+
+Starting with GitLab 9.1.0 it's possible to upgrade to a newer version of GitLab
+without having to take your GitLab instance offline. However, for this to work
+there are the following requirements:
+
+1. You can only upgrade 1 release at a time. For example, if 9.1.15 is the last
+   release of 9.1 then you can safely upgrade from that version to 9.2.0.
+   However, if you are running 9.1.14 you first need to upgrade to 9.1.15.
+2. You have to use [post-deployment
+   migrations](../development/post_deployment_migrations.md).
+3. You are using PostgreSQL. If you are using MySQL you will still need downtime
+   when upgrading.
+
+This applies to major, minor, and patch releases unless stated otherwise in a
+release post.
+
 ## Upgrading between editions

 GitLab comes in two flavors: [Community Edition][ce] which is MIT licensed,
--- a/doc/user/project/cycle_analytics.md
+++ b/doc/user/project/cycle_analytics.md
@ -5,10 +5,10 @@

 Cycle Analytics measures the time it takes to go from an [idea to production] for
 each project you have. This is achieved by not only indicating the total time it
-takes to reach at that point, but the total time is broken down into the
+takes to reach that point, but the total time is broken down into the
 multiple stages an idea has to pass through to be shipped.

-Cycle Analytics is that it is tightly coupled with the [GitLab flow] and
+Cycle Analytics is tightly coupled with the [GitLab flow] and
 calculates a separate median for each stage.

 ## Overview
--- a/doc/user/project/integrations/kubernetes.md
+++ b/doc/user/project/integrations/kubernetes.md
@ -60,7 +60,7 @@ to use terminals. Support is currently limited to the first container in the
 first pod of your environment.

 When enabled, the Kubernetes service adds [web terminal](../../../ci/environments.md#web-terminals)
-support to your environments. This is based on the `exec` functionality found in
+support to your [environments](../../../ci/environments.md). This is based on the `exec` functionality found in
 Docker and Kubernetes, so you get a new shell session within your existing
 containers. To use this integration, you should deploy to Kubernetes using
 the deployment variables above, ensuring any pods you create are labelled with
--- a/lib/api/groups.rb
+++ b/lib/api/groups.rb
@ -5,11 +5,16 @@ module API
    before { authenticate! }

    helpers do
-      params :optional_params do
+      params :optional_params_ce do
        optional :description, type: String, desc: 'The description of the group'
        optional :visibility, type: String, values: Gitlab::VisibilityLevel.string_values, desc: 'The visibility of the group'
        optional :lfs_enabled, type: Boolean, desc: 'Enable/disable LFS for the projects in this group'
        optional :request_access_enabled, type: Boolean, desc: 'Allow users to request member access'
+        optional :share_with_group_lock, type: Boolean, desc: 'Prevent sharing a project with another group within this group'
+      end
+
+      params :optional_params do
+        use :optional_params_ce
      end

      params :statistics_params do
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@ -30,7 +30,7 @@ module API
        use :pagination
      end

-      params :issue_params do
+      params :issue_params_ce do
        optional :description, type: String, desc: 'The description of an issue'
        optional :assignee_id, type: Integer, desc: 'The ID of a user to assign issue'
        optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign issue'
@ -38,6 +38,10 @@ module API
        optional :due_date, type: String, desc: 'Date time string in the format YEAR-MONTH-DAY'
        optional :confidential, type: Boolean, desc: 'Boolean parameter if the issue should be confidential'
      end
+
+      params :issue_params do
+        use :issue_params_ce
+      end
    end

    resource :issues do
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@ -33,13 +33,17 @@ module API
          end
        end

-        params :optional_params do
+        params :optional_params_ce do
          optional :description, type: String, desc: 'The description of the merge request'
          optional :assignee_id, type: Integer, desc: 'The ID of a user to assign the merge request'
          optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
          optional :labels, type: String, desc: 'Comma-separated list of label names'
          optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
        end
+
+        params :optional_params do
+          use :optional_params_ce
+        end
      end

      desc 'List merge requests' do
@ -145,14 +149,24 @@ module API
        success Entities::MergeRequest
      end
      params do
+        # CE
+        at_least_one_of_ce = [
+          :assignee_id,
+          :description,
+          :labels,
+          :milestone_id,
+          :remove_source_branch,
+          :state_event,
+          :target_branch,
+          :title
+        ]
        optional :title, type: String, allow_blank: false, desc: 'The title of the merge request'
        optional :target_branch, type: String, allow_blank: false, desc: 'The target branch'
        optional :state_event, type: String, values: %w[close reopen],
                               desc: 'Status of the merge request'
+
        use :optional_params
-        at_least_one_of :title, :target_branch, :description, :assignee_id,
-                        :milestone_id, :labels, :state_event,
-                        :remove_source_branch
+        at_least_one_of(*at_least_one_of_ce)
      end
      put ':id/merge_requests/:merge_request_iid' do
        merge_request = find_merge_request_with_access(params.delete(:merge_request_iid), :update_merge_request)
@ -173,6 +187,7 @@ module API
        success Entities::MergeRequest
      end
      params do
+        # CE
        optional :merge_commit_message, type: String, desc: 'Custom merge commit message'
        optional :should_remove_source_branch, type: Boolean,
                                               desc: 'When true, the source branch will be deleted if possible'
--- a/Show More
+++ b/Show More