Skip to content

Modernize tests & linting #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Dec 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
inherit_gem:
jekyll: .rubocop.yml

AllCops:
TargetRubyVersion: 2.1
Exclude:
- vendor/**/*

Lint/ShadowingOuterLocalVariable:
Exclude:
- lib/jekyll-archives.rb

Metrics/BlockLength:
Exclude:
- test/**/*.rb

Metrics/LineLength:
Exclude:
- test/**/*.rb
- lib/jekyll-archives.rb
- lib/jekyll-archives/archive.rb
- test/**/*.rb
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ rvm:

env:
- ""
- JEKYLL_VERSION=3.4
- JEKYLL_VERSION=3.6

matrix:
include:
Expand Down
42 changes: 2 additions & 40 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
# frozen_string_literal: true

# From jekyll/jekyll-mentions

require "rubygems"
require "bundler"
require "bundler/gem_tasks"

begin
Bundler.setup(:default, :development, :test)
Expand All @@ -21,44 +20,7 @@ require "rake/testtask"
Rake::TestTask.new(:test) do |test|
test.libs << "lib" << "test"
test.pattern = "test/**/test_*.rb"
test.warning = false
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do? Suppress ruby warnings in the test output?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes!

end

task :default => "test"

# Release task

def name
@name ||= File.basename(Dir["*.gemspec"].first, ".*")
end

def version
Jekyll::Archives::VERSION
end

def gemspec_file
"#{name}.gemspec"
end

def gem_file
"#{name}-#{version}.gem"
end

desc "Release #{name} v#{version}"
task :release => :build do
unless `git branch` =~ %r!^\* master$!
puts "You must be on the master branch to release!"
exit!
end
sh "git commit --allow-empty -m 'Release :gem: #{version}'"
sh "git tag v#{version}"
sh "git push origin master"
sh "git push origin v#{version}"
sh "gem push pkg/#{name}-#{version}.gem"
end

desc "Build #{name} v#{version} into pkg/"
task :build do
mkdir_p "pkg"
sh "gem build #{gemspec_file}"
sh "mv #{gem_file} pkg"
end
6 changes: 4 additions & 2 deletions jekyll-archives.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ Gem::Specification.new do |s|

s.homepage = "https://github.com/jekyll/jekyll-archives"
s.licenses = ["MIT"]
s.files = ["lib/jekyll-archives.rb", "lib/jekyll-archives/archive.rb"]

s.add_dependency "jekyll", ">= 2.4"
all_files = `git ls-files -z`.split("\x0")
s.files = all_files.grep(%r!^(lib)/!)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use Dir globbing instead of git to get the file list? There's a growing practice of folks not depending on git to get the file list for gems in order to avoid putting git on production like systems.


s.add_dependency "jekyll", ">= 3.1"

s.add_development_dependency "minitest"
s.add_development_dependency "rake"
Expand Down
6 changes: 1 addition & 5 deletions lib/jekyll-archives.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ class Archives < Jekyll::Generator
}.freeze

def initialize(config = nil)
@config = if config["jekyll-archives"].nil?
DEFAULTS
else
Utils.deep_merge_hashes(DEFAULTS, config["jekyll-archives"])
end
@config = Utils.deep_merge_hashes(DEFAULTS, config.fetch("jekyll-archives", {}))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️
Hash#fetch is my jam

end

def generate(site)
Expand Down
4 changes: 4 additions & 0 deletions script/bootstrap
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
set -ex

bundle install
10 changes: 8 additions & 2 deletions script/fmt
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
#! /bin/sh
#!/bin/bash
set -e

bundle exec rubocop -S -D -E
echo "Rubocop $(bundle exec rubocop --version)"
bundle exec rubocop -S -D -E $@
success=$?
if ((success != 0)); then
echo -e "\nTry running \`script/fmt -a\` to automatically fix errors"
fi
exit $success
7 changes: 7 additions & 0 deletions script/release
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#!/bin/sh
# Tag and push a release.

set -e

script/cibuild
bundle exec rake release