[ci skip] Merge branch 'master' into 44427-state-management-with-vuex

* master: (544 commits)
  Bulk deleting refs is handled by Gitaly by default
  Allow assigning and filtering issuables by ancestor group labels
  Fix links to subdirectories of a directory with a plus character in its path
  Add banzai filter to detect commit message trailers and properly link the users
  Render MR commit SHA instead "diffs" when viable
  Fix a transient failure by removing unneeded expectations
  Revert changelog entry for removed feature
  Revert "Allow CI/CD Jobs being grouped on version strings"
  Add support for Sidekiq JSON logging
  Resolve "Protected branches count is wrong when a wildcard includes several protected branches"
  Remove unused form for admin application settings
  Use standard codequality job
  Resolve "Allow the configuration of a project's merge method via the API"
  Move the rest of application settings to expandable blocks
  [Rails5] Rename `sort` methods to `sort_by_attribute`
  Add better LDAP connection handling
  Updated components to PascalCase
  Handle invalid params when trying update_username
  Move network related app settings to expandable blocks
  [Rails5] Update Gemfile.rails5.lock [ci skip]
  ...

4 files changed, 491 insertions, 6 deletions

diff --git a/doc/development/ee_features.md b/doc/development/ee_features.md
index fea92e740cb..287143d6255 100644
--- a/doc/development/ee_features.md
+++ b/doc/development/ee_features.md
@@ -33,6 +33,26 @@ rest of the code should be as close to the CE files as possible.
 
 [single code base]: https://gitlab.com/gitlab-org/gitlab-ee/issues/2952#note_41016454
 
+### EE-specific comments
+
+When complete separation can't be achieved with the `ee/` directory, you can wrap
+code in EE specific comments to designate the difference from CE/EE and add
+some context for someone resolving a conflict.
+
+```rb
+# EE-specific start
+stub_licensed_features(variable_environment_scope: true)
+# EE specific end
+```
+
+```haml
+-# EE-specific start
+= render 'ci/variables/environment_scope', form_field: form_field, variable: variable
+-# EE-specific end
+```
+
+EE-specific comments should not be backported to CE.
+
 ### Detection of EE-only files
 
 For each commit (except on `master`), the `ee-files-location-check` CI job tries
@@ -350,6 +370,255 @@ class beneath the `EE` module just as you would normally.
 For example, if CE has LDAP classes in `lib/gitlab/ldap/` then you would place
 EE-specific LDAP classes in `ee/lib/ee/gitlab/ldap`.
 
+### Code in `lib/api/`
+
+It can be very tricky to extend EE features by a single line of `prepend`,
+and for each different [Grape](https://github.com/ruby-grape/grape) feature,
+we might need different strategies to extend it. To apply different strategies
+easily, we would use `extend ActiveSupport::Concern` in the EE module.
+
+Put the EE module files following
+[EE features based on CE features](#ee-features-based-on-ce-features).
+
+#### EE API routes
+
+For EE API routes, we put them in a `prepended` block:
+
+``` ruby
+module EE
+  module API
+    module MergeRequests
+      extend ActiveSupport::Concern
+
+      prepended do
+        params do
+          requires :id, type: String, desc: 'The ID of a project'
+        end
+        resource :projects, requirements: ::API::API::PROJECT_ENDPOINT_REQUIREMENTS do
+          # ...
+        end
+      end
+    end
+  end
+end
+```
+
+Note that due to namespace differences, we need to use the full qualifier for some
+constants.
+
+#### EE params
+
+We can define `params` and utilize `use` in another `params` definition to
+include params defined in EE. However, we need to define the "interface" first
+in CE in order for EE to override it. We don't have to do this in other places
+due to `prepend`, but Grape is complex internally and we couldn't easily do
+that, so we'll follow regular object-oriented practices that we define the
+interface first here.
+
+For example, suppose we have a few more optional params for EE, given this CE
+API code:
+
+``` ruby
+module API
+  class MergeRequests < Grape::API
+    # EE::API::MergeRequests would override the following helpers
+    helpers do
+      params :optional_params_ee do
+      end
+    end
+
+    prepend EE::API::MergeRequests
+
+    params :optional_params do
+      # CE specific params go here...
+
+      use :optional_params_ee
+    end
+  end
+end
+```
+
+And then we could override it in EE module:
+
+``` ruby
+module EE
+  module API
+    module MergeRequests
+      extend ActiveSupport::Concern
+
+      prepended do
+        helpers do
+          params :optional_params_ee do
+            # EE specific params go here...
+          end
+        end
+      end
+    end
+  end
+end
+```
+
+This way, the only difference between CE and EE for that API file would be
+`prepend EE::API::MergeRequests`.
+
+#### EE helpers
+
+To make it easy for an EE module to override the CE helpers, we need to define
+those helpers we want to extend first. Try to do that immediately after the
+class definition to make it easy and clear:
+
+``` ruby
+module API
+  class JobArtifacts < Grape::API
+    # EE::API::JobArtifacts would override the following helpers
+    helpers do
+      def authorize_download_artifacts!
+        authorize_read_builds!
+      end
+    end
+
+    prepend EE::API::JobArtifacts
+  end
+end
+```
+
+And then we can follow regular object-oriented practices to override it:
+
+``` ruby
+module EE
+  module API
+    module JobArtifacts
+      extend ActiveSupport::Concern
+
+      prepended do
+        helpers do
+          def authorize_download_artifacts!
+            super
+            check_cross_project_pipelines_feature!
+          end
+        end
+      end
+    end
+  end
+end
+```
+
+#### EE-specific behaviour
+
+Sometimes we need EE-specific behaviour in some of the APIs. Normally we could
+use EE methods to override CE methods, however API routes are not methods and
+therefore can't be simply overridden. We need to extract them into a standalone
+method, or introduce some "hooks" where we could inject behavior in the CE
+route. Something like this:
+
+``` ruby
+module API
+  class MergeRequests < Grape::API
+    helpers do
+      # EE::API::MergeRequests would override the following helpers
+      def update_merge_request_ee(merge_request)
+      end
+    end
+
+    prepend EE::API::MergeRequests
+
+    put ':id/merge_requests/:merge_request_iid/merge' do
+      merge_request = find_project_merge_request(params[:merge_request_iid])
+
+      # ...
+
+      update_merge_request_ee(merge_request)
+
+      # ...
+    end
+  end
+end
+```
+
+Note that `update_merge_request_ee` doesn't do anything in CE, but
+then we could override it in EE:
+
+``` ruby
+module EE
+  module API
+    module MergeRequests
+      extend ActiveSupport::Concern
+
+      prepended do
+        helpers do
+          def update_merge_request_ee(merge_request)
+            # ...
+          end
+        end
+      end
+    end
+  end
+end
+```
+
+#### EE `route_setting`
+
+It's very hard to extend this in an EE module, and this is simply storing
+some meta-data for a particular route. Given that, we could simply leave the
+EE `route_setting` in CE as it won't hurt and we are just not going to use
+those meta-data in CE.
+
+We could revisit this policy when we're using `route_setting` more and whether
+or not we really need to extend it from EE. For now we're not using it much.
+
+#### Utilizing class methods for setting up EE-specific data
+
+Sometimes we need to use different arguments for a particular API route, and we
+can't easily extend it with an EE module because Grape has different context in
+different blocks. In order to overcome this, we could use class methods from the
+API class.
+
+For example, in one place we need to pass an extra argument to
+`at_least_one_of` so that the API could consider an EE-only argument as the
+least argument. This is not quite beautiful but it's working:
+
+``` ruby
+module API
+  class MergeRequests < Grape::API
+    def self.update_params_at_least_one_of
+      %i[
+        assignee_id
+        description
+      ]
+    end
+
+    prepend EE::API::MergeRequests
+
+    params do
+      at_least_one_of(*::API::MergeRequests.update_params_at_least_one_of)
+    end
+  end
+end
+```
+
+And then we could easily extend that argument in the EE class method:
+
+``` ruby
+module EE
+  module API
+    module MergeRequests
+      extend ActiveSupport::Concern
+
+      class_methods do
+        def update_params_at_least_one_of
+          super.push(*%i[
+            squash
+          ])
+        end
+      end
+    end
+  end
+end
+```
+
+It could be annoying if we need this for a lot of routes, but it might be the
+simplest solution right now.
+
 ### Code in `spec/`
 
 When you're testing EE-only features, avoid adding examples to the
@@ -375,6 +644,7 @@ information on managing page-specific javascript within EE.
 To separate EE-specific styles in SCSS files, if a component you're adding styles for
 is limited to only EE, it is better to have a separate SCSS file in appropriate directory
 within `app/assets/stylesheets`.
+See [backporting changes](#backporting-changes) for instructions on how to merge changes safely.
 
 In some cases, this is not entirely possible or creating dedicated SCSS file is an overkill,
 e.g. a text style of some component is different for EE. In such cases,
@@ -405,14 +675,28 @@ to avoid conflicts during CE to EE merge.
   }
 }
 
-/* EE-specific styles */
+// EE-specific start
 .section-body.ee-section-body {
   .section-title {
     background: $gl-header-color-cyan;
   }
 }
+// EE-specific end
 ```
 
+### Backporting changes from EE to CE
+
+When working in EE-specific features, you might have to tweak a few files that are not EE-specific. Here is a workflow to make sure those changes end up backported safely into CE too.
+(This approach does not refer to changes introduced via [csslab](https://gitlab.com/gitlab-org/csslab/).)
+
+1. **Make your changes in the EE branch.** If possible, keep a separated commit (to be squashed) to help backporting and review.
+1. **Open merge request to EE project.**
+1. **Apply the changes you made to CE files in a branch of the CE project.** (Tip: Use `patch` with the diff from your commit in EE branch)
+1. **Open merge request to CE project**, referring it's a backport of EE changes and link to MR open in EE.
+1. Once EE MR is merged, the MR towards CE can be merged. **But not before**.
+
+**Note:** regarding SCSS, make sure the files living outside `/ee/` don't diverge between CE and EE projects.
+
 ## gitlab-svgs
 
 Conflicts in `app/assets/images/icons.json` or `app/assets/images/icons.svg` can
diff --git a/doc/development/i18n/proofreader.md b/doc/development/i18n/proofreader.md
index 960eabd5538..cf62314bc29 100644
--- a/doc/development/i18n/proofreader.md
+++ b/doc/development/i18n/proofreader.md
@@ -10,6 +10,7 @@ are very appreciative of the work done by translators and proofreaders!
   - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve)
 - Chinese Traditional
   - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve)
+  - Weizhe Ding - [GitLab](https://gitlab.com/d.weizhe), [Crowdin](https://crowdin.com/profile/d.weizhe)
 - Chinese Traditional, Hong Kong
   - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve)
 - Dutch
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index 1e060ffd941..a211effdfa7 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -23,10 +23,6 @@ When downtime is necessary the migration has to be approved by:
 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 few assumptions as
 possible about the state of the database.
@@ -41,6 +37,18 @@ Migrations that make changes to the database schema (e.g. adding a column) can
 only be added in the monthly release, patch releases may only contain data
 migrations _unless_ schema changes are absolutely required to solve a problem.
 
+## What Requires Downtime?
+
+The document ["What Requires Downtime?"](what_requires_downtime.md) specifies
+various database operations, such as 
+
+- [adding, dropping, and renaming columns](what_requires_downtime.md#adding-columns)
+- [changing column constraints and types](what_requires_downtime.md#changing-column-constraints)
+- [adding and dropping indexes, tables, and foreign keys](what_requires_downtime.md#adding-indexes)
+
+and whether they require downtime and how to work around that whenever possible.
+
+
 ## Downtime Tagging
 
 Every migration must specify if it requires downtime or not, and if it should
diff --git a/doc/development/new_fe_guide/style/javascript.md b/doc/development/new_fe_guide/style/javascript.md
index 480d50a211f..57efd9353bc 100644
--- a/doc/development/new_fe_guide/style/javascript.md
+++ b/doc/development/new_fe_guide/style/javascript.md
@@ -1,3 +1,195 @@
 # JavaScript style guide
 
-> TODO: Add content
+We use [Airbnb's JavaScript Style Guide][airbnb-style-guide] and it's accompanying linter to manage most of our JavaScript style guidelines.
+
+In addition to the style guidelines set by Airbnb, we also have a few specific rules listed below.
+
+> **Tip:**
+You can run eslint locally by running `yarn eslint`
+
+## Arrays
+
+<a name="avoid-foreach"></a><a name="1.1"></a>
+- [1.1](#avoid-foreach) **Avoid ForEach when mutating data** Use `map`, `reduce` or `filter` instead of `forEach` when mutating data. This will minimize mutations in functions ([which is aligned with Airbnb's style guide][airbnb-minimize-mutations])
+
+```
+// bad
+users.forEach((user, index) => {
+  user.id = index;
+});
+
+// good
+const usersWithId = users.map((user, index) => {
+  return Object.assign({}, user, { id: index });
+});
+```
+
+## Functions
+
+<a name="limit-params"></a><a name="2.1"></a>
+- [2.1](#limit-params) **Limit number of parameters** If your function or method has more than 3 parameters, use an object as a parameter instead.
+
+```
+// bad
+function a(p1, p2, p3) {
+  // ...
+};
+
+// good
+function a(p) {
+  // ...
+};
+```
+
+## Classes & constructors
+
+<a name="avoid-constructor-side-effects"></a><a name="3.1"></a>
+- [3.1](#avoid-constructor-side-effects) **Avoid side effects in constructors** Avoid making some operations in the `constructor`, such as asynchronous calls, API requests and DOM manipulations. Prefer moving them into separate functions. This will make tests easier to write and code easier to maintain.
+
+  ```javascript
+  // bad
+  class myClass {
+    constructor(config) {
+      this.config = config;
+      axios.get(this.config.endpoint)
+    }
+  }
+
+  // good
+  class myClass {
+    constructor(config) {
+      this.config = config;
+    }
+
+    makeRequest() {
+      axios.get(this.config.endpoint)
+    }
+  }
+  const instance = new myClass();
+  instance.makeRequest();
+
+  ```
+
+<a name="avoid-classes-to-handle-dom-events"></a><a name="3.2"></a>
+- [3.2](#avoid-classes-to-handle-dom-events) **Avoid classes to handle DOM events** If the only purpose of the class is to bind a DOM event and handle the callback, prefer using a function.
+
+```
+// bad
+class myClass {
+  constructor(config) {
+    this.config = config;
+  }
+
+  init() {
+    document.addEventListener('click', () => {});
+  }
+}
+
+// good
+
+const myFunction = () => {
+  document.addEventListener('click', () => {
+    // handle callback here
+  });
+}
+```
+
+<a name="element-container"></a><a name="3.3"></a>
+- [3.3](#element-container) **Pass element container to constructor** When your class manipulates the DOM, receive the element container as a parameter.
+This is more maintainable and performant.
+
+```
+// bad
+class a {
+  constructor() {
+    document.querySelector('.b');
+  }
+}
+
+// good
+class a {
+  constructor(options) {
+    options.container.querySelector('.b');
+  }
+}
+```
+
+## Type Casting & Coercion
+
+<a name="use-parseint"></a><a name="4.1"></a>
+- [4.1](#use-parseint) **Use ParseInt** Use `ParseInt` when converting a numeric string into a number.
+
+```
+// bad
+Number('10')
+
+
+// good
+parseInt('10', 10);
+```
+
+## CSS Selectors
+
+<a name="use-js-prefix"></a><a name="5.1"></a>
+- [5.1](#use-js-prefix) **Use js prefix** If a CSS class is only being used in JavaScript as a reference to the element, prefix the class name with `js-`
+
+```
+// bad
+<button class="add-user"></button>
+
+// good
+<button class="js-add-user"></button>
+```
+
+## Modules
+
+<a name="use-absolute-paths"></a><a name="6.1"></a>
+- [6.1](#use-absolute-paths) **Use absolute paths for nearby modules** Use absolute paths if the module you are importing is less than two levels up.
+
+```
+// bad
+import GitLabStyleGuide from '~/guides/GitLabStyleGuide';
+
+// good
+import GitLabStyleGuide from '../GitLabStyleGuide';
+```
+
+<a name="use-relative-paths"></a><a name="6.2"></a>
+- [6.2](#use-relative-paths) **Use relative paths for distant modules** If the module you are importing is two or more levels up, use a relative path instead of an absolute path.
+
+```
+// bad
+import GitLabStyleGuide from '../../../guides/GitLabStyleGuide';
+
+// good
+import GitLabStyleGuide from '~/GitLabStyleGuide';
+```
+
+<a name="global-namespace"></a><a name="6.3"></a>
+- [6.3](#global-namespace) **Do not add to global namespace**
+
+<a name="domcontentloaded"></a><a name="6.4"></a>
+- [6.4](#domcontentloaded) **Do not use DOMContentLoaded in non-page modules** Imported modules should act the same each time they are loaded. `DOMContentLoaded` events are only allowed on modules loaded in the `/pages/*` directory because those are loaded dynamically with webpack.
+
+## Security
+
+<a name="avoid-xss"></a><a name="7.1"></a>
+- [7.1](#avoid-xss) **Avoid XSS** Do not use `innerHTML`, `append()` or `html()` to set content. It opens up too many vulnerabilities.
+
+## ESLint
+
+<a name="disable-eslint-file"></a><a name="8.1"></a>
+- [8.1](#disable-eslint-file) **Disabling ESLint in new files** Do not disable ESLint when creating new files. Existing files may have existing rules disabled due to legacy compatibility reasons but they are in the process of being refactored.
+
+<a name="disable-eslint-rule"></a><a name="8.2"></a>
+- [8.2](#disable-eslint-rule) **Disabling ESLint rule** Do not disable specific ESLint rules. Due to technical debt, you may disable the following rules only if you are invoking/instantiating existing code modules
+
+  - [no-new][no-new]
+  - [class-method-use-this][class-method-use-this]
+
+> Note: Disable these rules on a per line basis. This makes it easier to refactor in the future. E.g. use `eslint-disable-next-line` or `eslint-disable-line`
+
+[airbnb-style-guide]: https://github.com/airbnb/javascript
+[airbnb-minimize-mutations]: https://github.com/airbnb/javascript#testing--for-real
+[no-new]: http://eslint.org/docs/rules/no-new
+[class-method-use-this]: http://eslint.org/docs/rules/class-methods-use-this