From 7ce507710018d3368b13f3c29820bfb061cdca24 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 3 Oct 2017 17:27:24 +0100 Subject: [PATCH 1/4] Add Rubocop and sensible default rules. --- .rubocop.yml | 27 +++++++++++++++++++++++++++ Gemfile | 1 + Gemfile.lock | 17 +++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100644 .rubocop.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 000000000000..78bc878098f2 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,27 @@ +AllCops: + TargetRubyVersion: 2.4 + Exclude: + - '**/bin/**/*' + - '**/db/**/*' + - '**/script/setup' + - '**/vendor/**/*' + +Lint/AssignmentInCondition: + Enabled: false + +Metrics/BlockLength: + Enabled: false + +Metrics/LineLength: + Max: 100 + IgnoredPatterns: ['\A\s*#'] + +Style/FrozenStringLiteralComment: + Enabled: false + +Style/StringLiterals: + EnforcedStyle: double_quotes + +Style/TrailingCommaInLiteral: + EnforcedStyleForMultiline: consistent_comma + diff --git a/Gemfile b/Gemfile index 1c1c0e181f12..e6a3f1f7973c 100644 --- a/Gemfile +++ b/Gemfile @@ -3,5 +3,6 @@ source "https://rubygems.org" group :test do gem "minitest", "~> 5.10.3" gem "rake" + gem "rubocop", "~> 0.50.0" gem "safe_yaml", "~> 1.0.4" end diff --git a/Gemfile.lock b/Gemfile.lock index 052618af6259..0b30e41424f8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,9 +1,25 @@ GEM remote: https://rubygems.org/ specs: + ast (2.3.0) minitest (5.10.3) + parallel (1.12.0) + parser (2.4.0.0) + ast (~> 2.2) + powerpack (0.1.1) + rainbow (2.2.2) + rake rake (12.0.0) + rubocop (0.50.0) + parallel (~> 1.10) + parser (>= 2.3.3.1, < 3.0) + powerpack (~> 0.1) + rainbow (>= 2.2.2, < 3.0) + ruby-progressbar (~> 1.7) + unicode-display_width (~> 1.0, >= 1.0.1) + ruby-progressbar (1.9.0) safe_yaml (1.0.4) + unicode-display_width (1.3.0) PLATFORMS ruby @@ -11,6 +27,7 @@ PLATFORMS DEPENDENCIES minitest (~> 5.10.3) rake + rubocop (~> 0.50.0) safe_yaml (~> 1.0.4) BUNDLED WITH From a9d13ec1377b405d1f9d7d872e48c1f95242b198 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 3 Oct 2017 17:28:18 +0100 Subject: [PATCH 2/4] Fix all Rubocop warnings. --- Rakefile | 2 +- test/test_helper.rb | 12 ++++++------ test/topics_test.rb | 12 +++++++----- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Rakefile b/Rakefile index 172685d948c2..5dfa61083876 100644 --- a/Rakefile +++ b/Rakefile @@ -7,4 +7,4 @@ Rake::TestTask.new do |t| t.verbose = false end -task :default => :test +task default: :test diff --git a/test/test_helper.rb b/test/test_helper.rb index 1911b181321b..036d9710f7cd 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,12 +1,12 @@ require "minitest/autorun" require "yaml" -IMAGE_EXTENSIONS = [".jpg", ".jpeg", ".png"].freeze +IMAGE_EXTENSIONS = %w[.jpg .jpeg .png].freeze -VALID_METADATA_KEYS = ["aliases", "created_by", "display_name", "github_url", "logo", "related", - "released", "short_description", "topic", "url", "wikipedia_url"].freeze +VALID_METADATA_KEYS = %w[aliases created_by display_name github_url logo related + released short_description topic url wikipedia_url].freeze -REQUIRED_METADATA_KEYS = ["topic", "short_description"].freeze +REQUIRED_METADATA_KEYS = %w[topic short_description].freeze def topics_dir File.expand_path("../topics", File.dirname(__FILE__)) @@ -39,8 +39,8 @@ def metadata_for(topic) parts = File.read(path).split("---", 3) return unless parts.size >= 2 - metadata = begin - YAML.load(parts[1]) + begin + YAML.safe_load(parts[1]) rescue Psych::SyntaxError => ex flunk "invalid YAML: #{ex.message}" end diff --git a/test/topics_test.rb b/test/topics_test.rb index 107a6e566a1e..2f828c84d7bc 100644 --- a/test/topics_test.rb +++ b/test/topics_test.rb @@ -16,7 +16,7 @@ if path = paths.first assert_equal topic, File.basename(path, File.extname(path)), - "expected image to be named [topic].[extension]" + "expected image to be named [topic].[extension]" end end @@ -38,7 +38,7 @@ if File.file?(path) lines = File.readlines(path) - assert lines.size > 0 + assert !lines.empty? assert_equal "---\n", lines[0], "expected file to start with Jekyll front matter ---" end_index = lines.slice(1..-1).index("---\n") @@ -50,14 +50,16 @@ metadata = metadata_for(topic) refute_empty metadata, "expected some metadata for topic" - metadata.each do |key, value| + metadata.each_key do |key,| assert_includes VALID_METADATA_KEYS, key, "unexpected metadata key '#{key}'" end REQUIRED_METADATA_KEYS.each do |key| assert metadata.key?(key), "expected to have '#{key}' defined for topic" - assert metadata[key] && metadata[key].strip.size > 0, - "expected to have a value for '#{key}'" + assert metadata[key]&.strip&.size&.positive?, + "expected to have a value for '#{key}'" + end + end end end end From e86b48a48a64b546d1870c220819d71c17c826c3 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 3 Oct 2017 17:29:13 +0100 Subject: [PATCH 3/4] Use scripts-to-rule-them-all format --- .ruby-version | 1 + .travis.yml | 12 +++++------- Brewfile | 6 ++++++ README.md | 3 +-- script/cibuild | 14 ++++++++++++++ script/setup | 11 +++++++++++ 6 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 .ruby-version create mode 100644 Brewfile create mode 100755 script/cibuild create mode 100755 script/setup diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 000000000000..005119baaa06 --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.4.1 diff --git a/.travis.yml b/.travis.yml index 0d826c7e2145..85ff3f8817b4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,6 @@ +language: ruby +cache: bundler sudo: false -install: - - bundle install -notifications: - email: false -script: rake -cache: - bundler: true +bundler_args: --jobs=3 --retry=3 +before_install: ./script/setup +script: ./script/cibuild diff --git a/Brewfile b/Brewfile new file mode 100644 index 000000000000..be87f11be92f --- /dev/null +++ b/Brewfile @@ -0,0 +1,6 @@ +tap "github/bootstrap" + +brew "rbenv" +brew "ruby-build" + +brew "imagemagick" diff --git a/README.md b/README.md index 2645d371e7ff..66181b8ac593 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,5 @@ There are some lint tests in place to ensure each topic is formatted in the way run the tests using: ```bash -bundle install -rake +./script/cibuild ``` diff --git a/script/cibuild b/script/cibuild new file mode 100755 index 000000000000..89bb5f3d4fa7 --- /dev/null +++ b/script/cibuild @@ -0,0 +1,14 @@ +#!/bin/bash +set -e + +cd "$(dirname "$0")/.." + +./script/setup + +set +e + +bundle exec rake test +RAKE_EXIT="$?" +bundle exec rubocop --display-cop-names +RUBOCOP_EXIT="$?" +[[ "$RAKE_EXIT" == 0 && "$RUBOCOP_EXIT" == 0 ]] diff --git a/script/setup b/script/setup new file mode 100755 index 000000000000..2474f498f8c9 --- /dev/null +++ b/script/setup @@ -0,0 +1,11 @@ +#!/bin/bash +set -e + +cd "$(dirname "$0")/.." + +if [ "$(uname -s)" = "Darwin" ]; then + brew bundle check &>/dev/null || brew bundle + rbenv version-name &>/dev/null || brew bootstrap-rbenv-ruby +fi + +bundle check &>/dev/null || bundle install From 93e03ae1f497db76861ab0831227017d7e68ef97 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Tue, 3 Oct 2017 17:43:16 +0100 Subject: [PATCH 4/4] Address review feedback. --- test/topics_test.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/topics_test.rb b/test/topics_test.rb index 2f828c84d7bc..720c32ab4d0d 100644 --- a/test/topics_test.rb +++ b/test/topics_test.rb @@ -38,7 +38,7 @@ if File.file?(path) lines = File.readlines(path) - assert !lines.empty? + refute lines.empty? assert_equal "---\n", lines[0], "expected file to start with Jekyll front matter ---" end_index = lines.slice(1..-1).index("---\n") @@ -50,7 +50,7 @@ metadata = metadata_for(topic) refute_empty metadata, "expected some metadata for topic" - metadata.each_key do |key,| + metadata.each_key do |key| assert_includes VALID_METADATA_KEYS, key, "unexpected metadata key '#{key}'" end @@ -60,8 +60,6 @@ "expected to have a value for '#{key}'" end end - end - end end end end