From 1b7e69c074b63c8306b5624b46c608411d0b4704 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9my=20Coutable?= <remy@rymai.me>
Date: Fri, 22 Jan 2021 07:24:50 +0000
Subject: [PATCH] Add Lefthook to the Gemfile and improve documentation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This will ease the integration of Lefthook with GitPod.

Signed-off-by: Rémy Coutable <remy@rymai.me>
---
 Gemfile                                      |  1 +
 Gemfile.lock                                 |  2 +
 doc/development/contributing/style_guides.md | 93 +++++++++++++-------
 doc/development/documentation/testing.md     |  2 +-
 4 files changed, 66 insertions(+), 32 deletions(-)

diff --git a/Gemfile b/Gemfile
index 551cbd9d229e9..24af3d70e4ebe 100644
--- a/Gemfile
+++ b/Gemfile
@@ -336,6 +336,7 @@ end
 group :development do
   gem 'brakeman', '~> 4.2', require: false
   gem 'danger', '~> 8.0.6', require: false
+  gem 'lefthook', '~> 0.7', require: false
 
   gem 'letter_opener_web', '~> 1.3.4'
 
diff --git a/Gemfile.lock b/Gemfile.lock
index ee5cbc7e2a0b6..4c633981d206f 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -658,6 +658,7 @@ GEM
       rest-client (~> 2.0)
     launchy (2.4.3)
       addressable (~> 2.3)
+    lefthook (0.7.2)
     letter_opener (1.7.0)
       launchy (~> 2.2)
     letter_opener_web (1.3.4)
@@ -1410,6 +1411,7 @@ DEPENDENCIES
   knapsack (~> 1.17)
   kramdown (~> 2.3.0)
   kubeclient (~> 4.9.1)
+  lefthook (~> 0.7)
   letter_opener_web (~> 1.3.4)
   license_finder (~> 6.0)
   licensee (~> 8.9)
diff --git a/doc/development/contributing/style_guides.md b/doc/development/contributing/style_guides.md
index c316d50c88c57..3ed23dcc29894 100644
--- a/doc/development/contributing/style_guides.md
+++ b/doc/development/contributing/style_guides.md
@@ -15,52 +15,83 @@ settings automatically by default. If your editor/IDE does not automatically sup
 we suggest investigating to see if a plugin exists. For instance here is the
 [plugin for vim](https://github.com/editorconfig/editorconfig-vim).
 
-## Pre-push static analysis
+## Pre-push static analysis with Lefthook
 
-We strongly recommend installing [Lefthook](https://github.com/Arkweid/lefthook) to automatically check
-for static analysis offenses before pushing your changes.
+[Lefthook](https://github.com/Arkweid/lefthook) is a Git hooks manager that allows
+custom logic to be executed prior to Git committing or pushing. GitLab comes with
+Lefthook configuration (`lefthook.yml`), but it must be installed.
 
-To install `lefthook`, run the following in your GitLab source directory:
+We have a `lefthook.yml` checked in but it is ignored until Lefthook is installed.
 
-```shell
-# 1. Make sure to uninstall Overcommit first
-overcommit --uninstall
+### Uninstall Overcommit
+
+We were using Overcommit prior to Lefthook, so you may want to uninstall it first with `overcommit --uninstall`.
+
+### Install Lefthook
+
+1. Install the `lefthook` Ruby gem:
+
+   ```shell
+   bundle install
+   ```
+
+1. Install Lefthook managed Git hooks:
+
+   ```shell
+   bundle exec lefthook install
+   ```
+
+1. Test Lefthook is working by running the Lefthook `prepare-commit-msg` Git hook:
+
+   ```shell
+   bundle exec lefthook run prepare-commit-msg
+   ```
+
+This should return a fully qualified path command with no other output.
 
-# If using rbenv, at this point you may need to do: rbenv rehash
+### Lefthook configuration
 
-# 2. Install lefthook...
+The current Lefthook configuration can be found in [`lefthook.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lefthook.yml).
 
-## With Homebrew (macOS)
-brew install Arkweid/lefthook/lefthook
+Before you push your changes, Lefthook automatically runs the following checks:
 
-## Or with Go
-go get github.com/Arkweid/lefthook
+- Danger: Runs a subset of checks that `danger-review` runs on your merge requests.
+- ES lint: Run `yarn eslint` checks (with the [`.eslintrc.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.eslintrc.yml) config) on the modified `*.{js,vue}` files. Tags: `frontend`, `style`.
+- HAML lint: Run `bundle exec haml-lint` checks (with the [`.haml-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.haml-lint.yml) config) on the modified `*.html.haml` files. Tags: `view`, `haml`, `style`.
+- Markdown lint: Run `yarn markdownlint` checks on the modified `*.md` files. Tags: `documentation`, `style`.
+- SCSS lint: Run `bundle exec scss-lint` checks (with the [`.scss-lint.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.scss-lint.yml) config) on the modified `*.scss{,.css}` files. Tags: `stylesheet`, `css`, `style`.
+- RuboCop: Run `bundle exec rubocop` checks (with the [`.rubocop.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.rubocop.yml) config) on the modified `*.rb` files. Tags: `backend`, `style`.
+- Vale: Run `vale` checks (with the [`.vale.ini`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.vale.ini) config) on the modified `*.md` files. Tags: `documentation`, `style`.
 
-## Or with Rubygems
-gem install lefthook
+In addition to the default configuration, you can define a [local configuration](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#local-config).
 
-### You may need to run the following if you're using rbenv
-rbenv rehash
+### Disable Lefthook temporarily
 
-# 3. Install the Git hooks
-lefthook install -f
+To disable Lefthook temporarily, you can set the `LEFTHOOK` environment variable to `0`. For instance:
+
+```shell
+LEFTHOOK=0 git push ...
+```
+
+### Run Lefthook hooks manually
+
+To run the `pre-push` Git hook, run:
+
+```shell
+bundle exec lefthook run pre-push
 ```
 
-Before you push your changes, Lefthook then automatically run Danger checks, and other checks
-for changed files. This saves you time as you don't have to wait for the same errors to be detected
-by CI/CD.
+For more information, check out [Lefthook documentation](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#run-githook-group-directly).
+
+### Skip Lefthook checks per tag
 
-Lefthook relies on a pre-push hook to prevent commits that violate its ruleset.
-To override this behavior, pass the environment variable `LEFTHOOK=0`. That is,
-`LEFTHOOK=0 git push`.
+To skip some checks based on tags when pushing, you can set the `LEFTHOOK_EXCLUDE` environment variable. For instance:
 
-You can also:
+```shell
+LEFTHOOK_EXCLUDE=frontend,documentation git push ...
+```
 
-- Define [local configuration](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#local-config).
-- Skip [checks per tag on the fly](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#skip-some-tags-on-the-fly).
-  For example, `LEFTHOOK_EXCLUDE=frontend git push origin`.
-- Run [hooks manually](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#run-githook-group-directly).
-  For example, `lefthook run pre-push`.
+For more information, check out [Lefthook documentation](https://github.com/Arkweid/lefthook/blob/master/docs/full_guide.md#skip-some-tags-on-the-fly).
 
 ## Ruby, Rails, RSpec
 
diff --git a/doc/development/documentation/testing.md b/doc/development/documentation/testing.md
index 561727648f0ce..7526464a46e79 100644
--- a/doc/development/documentation/testing.md
+++ b/doc/development/documentation/testing.md
@@ -286,7 +286,7 @@ Configuration for `lefthook` is available in the [`lefthook.yml`](https://gitlab
 file for the [`gitlab`](https://gitlab.com/gitlab-org/gitlab) project.
 
 To set up `lefthook` for documentation linting, see
-[Pre-push static analysis](../contributing/style_guides.md#pre-push-static-analysis).
+[Pre-push static analysis](../contributing/style_guides.md#pre-push-static-analysis-with-lefthook).
 
 ### Show subset of Vale alerts
 
-- 
GitLab