From c6a90b266dd3f1550c2b9b0a709ee3b109f3af5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 2 Oct 2020 11:36:17 +0200 Subject: [PATCH 1/3] Use Pronto to check code conventions This way developers can run the checks on their machines and using `bundle exec` we guarantee the right versions of all our gems are being used; with Hound we can only use the versions supported by their service. When including the pronto-erb_lint gem, we're getting errors in development where our ERB does not follow the conventions Better HTML expects. Since we only use Better HTML because ERB Lint depends on it, and right now we are not ready to follow its conventions, we're disabling it. Note pronto depends on rugged, which requires CMake and pkg-config to build the `libgit2` library it depends on. CMake and pkg-config are installed by default in some GNU/Linux distributions like Ubuntu, but might not be installed on other systems, so we're adding them as development dependencies. --- .github/workflows/linters.yml | 22 ++++++++++++++ .hound.yml | 11 ------- CONTRIBUTING.md | 2 +- CONTRIBUTING_ES.md | 2 +- Gemfile | 5 +++ Gemfile.lock | 49 ++++++++++++++++++++++++++---- README.md | 3 +- README_ES.md | 2 +- config/initializers/better_html.rb | 5 +++ 9 files changed, 79 insertions(+), 22 deletions(-) create mode 100644 .github/workflows/linters.yml delete mode 100644 .hound.yml create mode 100644 config/initializers/better_html.rb diff --git a/.github/workflows/linters.yml b/.github/workflows/linters.yml new file mode 100644 index 000000000..2c45f49bb --- /dev/null +++ b/.github/workflows/linters.yml @@ -0,0 +1,22 @@ +name: linters +on: [pull_request] + +jobs: + linters: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v2 + - run: | + git fetch --no-tags --prune origin +refs/heads/*:refs/remotes/origin/* + - name: Setup Ruby + uses: ruby/setup-ruby@v1 + with: + bundler-cache: true + - name: Setup NPM + uses: actions/setup-node@v1 + - name: Install eslint + run: npm install --global eslint@6.0.1 + - name: Run Pronto + run: | + PRONTO_PULL_REQUEST_ID="$(jq --raw-output .number "$GITHUB_EVENT_PATH")" PRONTO_GITHUB_ACCESS_TOKEN="${{ github.token }}" bundle exec pronto run -f github_status github_pr -c origin/${{ github.base_ref }} diff --git a/.hound.yml b/.hound.yml deleted file mode 100644 index c7c92dcfe..000000000 --- a/.hound.yml +++ /dev/null @@ -1,11 +0,0 @@ -rubocop: - config_file: .rubocop.yml - version: 0.91.0 -scss: - config_file: .scss-lint.yml -erblint: - enabled: true - config_file: .erb-lint.yml -eslint: - enabled: true - config_file: .eslintrc.yml diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0bc2d1b98..36cc22737 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,7 +34,7 @@ If you'd like us to review your pull request in good spirits, please follow our * Include specs to test any changes you've made * Our CI will check whether the rest of the application is still working properly; check its build and make sure all tests are passing -* Your pull request will be automatically reviewed by Hound CI; fix any issues it reports +* Run `bundle exec pronto run` and fix any issues it reports; these issues will also be automatically reported on the pull request * Follow [the seven rules of a great commit message](https://chris.beams.io/posts/git-commit/) When we review your pull request and ask for changes, if you're proficient using `git rebase` edit existing commits instead of adding new ones. If you aren't proficient with `git rebase`, ignore this point. diff --git a/CONTRIBUTING_ES.md b/CONTRIBUTING_ES.md index f189ee3ef..d69cc6184 100644 --- a/CONTRIBUTING_ES.md +++ b/CONTRIBUTING_ES.md @@ -34,7 +34,7 @@ Si quieres que revisemos tu código con una sonrisa, por favor sigue nuestras co * Incluye tests para los cambios que hayas hecho * Los tests se ejecutarán automáticamente para comprobar que el resto de la aplicación sigue funcionando; asegúrate de que los tests pasan -* Tus cambios serán revisados automáticamente por Hound CI; arregla los problemas de los que informa (si es que hay alguno) +* Ejecuta `bundle exec pronto run` y arregla los problemas de los que informe (si es que hay alguno) * Sigue [las siete reglas para un gran mensaje de commit](https://chris.beams.io/posts/git-commit/) Cuando revisemos tu código y te pidamos que cambies alguna cosa, si tienes experiencia con `git rebase` edita los commits existentes en vez de añadir más. Si no tienes experiencia con `git rebase`, puedes saltarte este punto. diff --git a/Gemfile b/Gemfile index 2779aa9a7..68bdd66fd 100644 --- a/Gemfile +++ b/Gemfile @@ -102,6 +102,11 @@ group :development do gem "erb_lint", "~> 0.0.35", require: false gem "github_changelog_generator", "~> 1.15.2" gem "mdl", "~> 0.11.0", require: false + gem "pronto", "~> 0.11.0" + gem "pronto-erb_lint", "~> 0.1.5" + gem "pronto-eslint", "~> 0.11.0" + gem "pronto-rubocop", "~> 0.11.0" + gem "pronto-scss", "~> 0.11.0" gem "rubocop", "~> 0.91.0", require: false gem "rubocop-performance", "~> 1.7.1", require: false gem "rubocop-rails", "~> 2.6.0", require: false diff --git a/Gemfile.lock b/Gemfile.lock index e848da938..25c26c07e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -218,6 +218,10 @@ GEM smart_properties errbase (0.0.3) erubi (1.9.0) + eslintrb (2.1.0) + execjs + multi_json (>= 1.3) + rake execjs (2.7.0) factory_bot (4.8.2) activesupport (>= 3.0.0) @@ -226,10 +230,13 @@ GEM railties (>= 3.0.0) faker (1.8.7) i18n (>= 0.7) - faraday (1.0.1) + faraday (1.3.0) + faraday-net_http (~> 1.0) multipart-post (>= 1.2, < 3) + ruby2_keywords faraday-http-cache (2.2.0) faraday (>= 0.8) + faraday-net_http (1.0.1) ffi (1.13.1) font-awesome-sass (5.15.1) sassc (>= 1.11) @@ -252,6 +259,9 @@ GEM rainbow (>= 2.2.1) rake (>= 10.0) retriable (~> 3.0) + gitlab (4.17.0) + httparty (~> 0.18) + terminal-table (~> 1.5, >= 1.5.1) globalid (0.4.2) activesupport (>= 4.2.0) globalize (5.3.0) @@ -272,7 +282,7 @@ GEM highline (2.0.3) html_tokenizer (0.0.7) htmlentities (4.3.4) - httparty (0.17.0) + httparty (0.18.1) mime-types (~> 3.0) multi_xml (>= 0.5.2) httpi (2.4.5) @@ -349,7 +359,7 @@ GEM method_source (1.0.0) mime-types (3.3.1) mime-types-data (~> 3.2015) - mime-types-data (3.2020.0512) + mime-types-data (3.2020.1104) mimemagic (0.3.5) mini_mime (1.0.2) mini_portile2 (2.4.0) @@ -377,7 +387,7 @@ GEM multi_json (~> 1.3) multi_xml (~> 0.5) rack (>= 1.2, < 3) - octokit (4.18.0) + octokit (4.20.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) omniauth (1.9.1) @@ -417,6 +427,26 @@ GEM pg_search (2.3.4) activerecord (>= 5.2) activesupport (>= 5.2) + pronto (0.11.0) + gitlab (~> 4.4, >= 4.4.0) + httparty (>= 0.13.7) + octokit (~> 4.7, >= 4.7.0) + rainbow (>= 2.2, < 4.0) + rexml (~> 3.2) + rugged (>= 0.23.0, < 1.1.0) + thor (>= 0.20.3, < 2.0) + pronto-erb_lint (0.1.5) + erb_lint (~> 0.0.24) + pronto (> 0.9.0) + pronto-eslint (0.11.0) + eslintrb (~> 2.0, >= 2.0.0) + pronto (~> 0.11.0) + pronto-rubocop (0.11.0) + pronto (~> 0.11.0) + rubocop (>= 0.49.1, < 1.0) + pronto-scss (0.11.0) + pronto (~> 0.11.0) + scss_lint (~> 0.43, >= 0.43.0) public_suffix (4.0.6) puma (4.3.6) nio4r (~> 2.0) @@ -514,7 +544,9 @@ GEM rubocop-rspec (1.41.0) rubocop (>= 0.68.1) ruby-progressbar (1.10.1) + ruby2_keywords (0.0.4) rubyzip (2.3.0) + rugged (1.0.1) rvm1-capistrano3 (1.4.0) capistrano (~> 3.0) sshkit (>= 1.2) @@ -584,7 +616,7 @@ GEM unicode-display_width (~> 1.1, >= 1.1.1) terrapin (0.6.0) climate_control (>= 0.0.3, < 1.0) - thor (1.0.1) + thor (1.1.0) thread_safe (0.3.6) tilt (2.0.10) tins (1.25.0) @@ -704,6 +736,11 @@ DEPENDENCIES paranoia (~> 2.4.2) pg (~> 1.0.0) pg_search (~> 2.3.4) + pronto (~> 0.11.0) + pronto-erb_lint (~> 0.1.5) + pronto-eslint (~> 0.11.0) + pronto-rubocop (~> 0.11.0) + pronto-scss (~> 0.11.0) puma (~> 4.3.6) rails (= 5.2.4.4) rails-assets-leaflet! @@ -740,4 +777,4 @@ DEPENDENCIES wkhtmltopdf-binary (~> 0.12.4) BUNDLED WITH - 1.17.1 + 1.17.2 diff --git a/README.md b/README.md index f4bc8c37e..4c99de6c3 100644 --- a/README.md +++ b/README.md @@ -15,7 +15,6 @@ Citizen Participation and Open Government Application [![Coverage Status](https://coveralls.io/repos/github/consul/consul/badge.svg)](https://coveralls.io/github/consul/consul?branch=master) [![Crowdin](https://d322cqt584bo4o.cloudfront.net/consul/localized.svg)](https://crowdin.com/project/consul) [![License: AGPL v3](https://img.shields.io/badge/License-AGPL%20v3-blue.svg)](http://www.gnu.org/licenses/agpl-3.0) -[![Reviewed by Hound](https://img.shields.io/badge/Reviewed_by-Hound-8E64B0.svg)](https://houndci.com) [![Accessibility conformance](https://img.shields.io/badge/accessibility-WAI:AA-green.svg)](https://www.w3.org/WAI/eval/Overview) [![A11y issues checked with Rocket Validator](https://rocketvalidator.com/badges/checked_with_rocket_validator.svg?url=https://rocketvalidator.com)](https://rocketvalidator.com/opensource) @@ -39,7 +38,7 @@ You can access the main website of the project at [http://consulproject.org](htt **NOTE**: For more detailed instructions check the [docs](https://docs.consulproject.org) -Prerequisites: install git, Ruby 2.6.6, Node.js and PostgreSQL (>=9.4). +Prerequisites: install git, Ruby 2.6.6, CMake, pkg-config, Node.js and PostgreSQL (>=9.4). ```bash git clone https://github.com/consul/consul.git diff --git a/README_ES.md b/README_ES.md index f54f333fd..e533f32c2 100644 --- a/README_ES.md +++ b/README_ES.md @@ -36,7 +36,7 @@ Puedes acceder a la página principal del proyecto en [http://consulproject.org] **NOTA**: para unas instrucciones más detalladas consulta la [documentación](https://docs.consulproject.org) -Prerequisitos: tener instalado git, Ruby 2.6.6, Node.js y PostgreSQL (9.4 o superior). +Prerequisitos: tener instalado git, Ruby 2.6.6, CMake, pkg-config, Node.js y PostgreSQL (9.4 o superior). ```bash git clone https://github.com/consul/consul.git diff --git a/config/initializers/better_html.rb b/config/initializers/better_html.rb new file mode 100644 index 000000000..5948f7319 --- /dev/null +++ b/config/initializers/better_html.rb @@ -0,0 +1,5 @@ +if Rails.env.development? + BetterHtml.configure do |config| + config.template_exclusion_filter = proc { |filename| true } + end +end From 7cce24961edcaab24880e822377b16951b1c7842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 2 Oct 2020 14:57:20 +0200 Subject: [PATCH 2/3] Don't fail the build on "refactoring" offenses We're a bit more relaxed about these rules, particularly about the line length and so the build should still pass when these rules are broken. --- .pronto.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 .pronto.yml diff --git a/.pronto.yml b/.pronto.yml new file mode 100644 index 000000000..07cc95343 --- /dev/null +++ b/.pronto.yml @@ -0,0 +1,4 @@ +rubocop: + suggestions: true + severities: + refactor: info From 7c4c6d4ee0df5bee2fba5721d6f612eb02e5e6da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 5 Feb 2021 15:38:02 +0100 Subject: [PATCH 3/3] Make ESLint configuration compatible with Pronto pronto-eslint depends on eslintrb, which expects the `.eslintrc` file in JSON format. So we're migrating the `.eslintrc.yml` file to JSON and introducing a symbolic link so the `.eslintrc` file points to `.eslintrc.json`. We could also use pronto-eslint_npm, but unfortunately it's not maintained anymore and the latest version is not compatible with Pronto 0.11. --- .eslintrc | 1 + .eslintrc.json | 115 +++++++++++++++++++++++++++++++++++++++++++++++++ .eslintrc.yml | 85 ------------------------------------ 3 files changed, 116 insertions(+), 85 deletions(-) create mode 120000 .eslintrc create mode 100644 .eslintrc.json delete mode 100644 .eslintrc.yml diff --git a/.eslintrc b/.eslintrc new file mode 120000 index 000000000..2db2e5dcd --- /dev/null +++ b/.eslintrc @@ -0,0 +1 @@ +.eslintrc.json \ No newline at end of file diff --git a/.eslintrc.json b/.eslintrc.json new file mode 100644 index 000000000..59459812d --- /dev/null +++ b/.eslintrc.json @@ -0,0 +1,115 @@ +{ + "env": { + "browser": true, + "es6": false + }, + "extends": "eslint:recommended", + "globals": { + "$": "readonly", + "App": "readonly", + "AmsifySuggestags": "readonly", + "annotator": "readonly", + "c3": "readonly", + "CKEDITOR": "readonly", + "L": "readonly", + "Turbolinks": "readonly" + }, + "parserOptions": { + "ecmaVersion": 5 + }, + "rules": { + "array-bracket-spacing": "error", + "array-callback-return": "error", + "block-spacing": "error", + "brace-style": "error", + "comma-spacing": "error", + "computed-property-spacing": "error", + "curly": "error", + "dot-notation": "error", + "eol-last": "error", + "eqeqeq": [ + "error", + "always", + { + "null": "ignore" + } + ], + "func-call-spacing": "error", + "indent": [ + "error", + 2 + ], + "key-spacing": "error", + "keyword-spacing": "error", + "linebreak-style": "error", + "lines-between-class-members": "error", + "max-len": [ + "warn", + { + "code": 110 + } + ], + "no-array-constructor": "error", + "no-console": "error", + "no-multi-spaces": "error", + "no-multiple-empty-lines": [ + "error", + { + "max": 1 + } + ], + "no-param-reassign": "error", + "no-shadow": "error", + "no-spaced-func": "error", + "no-tabs": "error", + "no-trailing-spaces": "error", + "no-void": "error", + "no-whitespace-before-property": "error", + "object-curly-spacing": [ + "error", + "always", + { + "objectsInObjects": false + } + ], + "padded-blocks": [ + "error", + "never" + ], + "quotes": [ + "error", + "double", + { + "avoidEscape": true + } + ], + "semi": [ + "error", + "always" + ], + "semi-spacing": "error", + "space-before-blocks": "error", + "space-before-function-paren": [ + "error", + "never" + ], + "space-in-parens": "error", + "space-infix-ops": "error", + "space-unary-ops": "error", + "spaced-comment": [ + "error", + "always", + { + "markers": [ + "=" + ], + "exceptions": [ + "-" + ] + } + ], + "strict": "error", + "switch-colon-spacing": "error", + "yoda": "error" + } +} diff --git a/.eslintrc.yml b/.eslintrc.yml deleted file mode 100644 index ec7a54ac4..000000000 --- a/.eslintrc.yml +++ /dev/null @@ -1,85 +0,0 @@ -env: - browser: true - es6: false -extends: "eslint:recommended" -globals: - $: readonly - App: readonly - AmsifySuggestags: readonly - annotator: readonly - c3: readonly - CKEDITOR: readonly - L: readonly - Turbolinks: readonly -parserOptions: - ecmaVersion: 5 -rules: - array-bracket-spacing: error - array-callback-return: error - block-spacing: error - brace-style: error - comma-spacing: error - computed-property-spacing: error - curly: error - dot-notation: error - eol-last: error - eqeqeq: - - error - - always - - "null": ignore - func-call-spacing: error - indent: - - error - - 2 - key-spacing: error - keyword-spacing: error - linebreak-style: error - lines-between-class-members: error - max-len: - - warn - - code: 110 - no-array-constructor: error - no-console: error - no-multi-spaces: error - no-multiple-empty-lines: - - error - - max: 1 - no-param-reassign: error - no-shadow: error - no-spaced-func: error - no-tabs: error - no-trailing-spaces: error - no-void: error - no-whitespace-before-property: error - object-curly-spacing: - - error - - always - - objectsInObjects: false - padded-blocks: - - error - - never - quotes: - - error - - double - - avoidEscape: true - semi: - - error - - always - semi-spacing: error - space-before-blocks: error - space-before-function-paren: - - error - - never - space-in-parens: error - space-infix-ops: error - space-unary-ops: error - spaced-comment: - - error - - always - - markers: - - "=" - exceptions: - - "-" - strict: error - switch-colon-spacing: error - yoda: error