Skip to content
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

Rubocop and comment out unneeded gems #29

Merged
merged 4 commits into from
Aug 25, 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
58 changes: 58 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
inherit_from: .rubocop_todo.yml

# Turn on RSpec cops
require: rubocop-rspec

AllCops:
DisplayCopNames: true
Include:
- Rakefile
- config.ru
- lib/**/*.rake
Exclude:
- 'Gemfile.lock'
- '**/*.md'
- 'bin/**/*'
- 'config/**/*.yml'
- 'db/**/*'
- 'script/**/*'
- 'vendor/**/*'

# Turn on Rails cops
Rails:
Enabled: true

Metrics/BlockLength:
Exclude:
- 'spec/**/*'

# because this isn't 1994
Metrics/LineLength:
Max: 120

Metrics/MethodLength:
Max: 20

Layout/EmptyLinesAroundBlockBody:
Enabled: false

Layout/EmptyLinesAroundClassBody:
Enabled: false

Layout/EmptyLinesAroundModuleBody:
Enabled: false

Style/StringLiterals:
Enabled: false

Style/SymbolArray:
Enabled: false

RSpec/MultipleExpectations:
Exclude:
- 'spec/controllers/moab_storage_controller_spec.rb'


Bundler/OrderedGems:
Exclude:
- 'Gemfile'
49 changes: 49 additions & 0 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# This configuration was generated by
# `rubocop --auto-gen-config`
# on 2017-08-23 17:50:12 -0700 using RuboCop version 0.49.1.
# The point is for the user to remove these configuration records
# one by one as the offenses are removed from the code base.
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 2
# Cop supports --auto-correct.
Layout/SpaceInsideBrackets:
Exclude:
- 'config/environments/production.rb'

# Offense count: 1
Rails/FilePath:
Exclude:
- 'config/environments/development.rb'

# Offense count: 1
# Cop supports --auto-correct.
Style/BlockComments:
Exclude:
- 'spec/spec_helper.rb'

# Offense count: 5
Style/Documentation:
Exclude:
- 'spec/**/*'
- 'test/**/*'
- 'app/controllers/application_controller.rb'
- 'app/helpers/application_helper.rb'
- 'app/mailers/application_mailer.rb'
- 'app/models/application_record.rb'
- 'config/application.rb'

# Offense count: 1
# Configuration parameters: ExpectMatchingDefinition, Regex, IgnoreExecutableScripts, AllowedAcronyms.
# AllowedAcronyms: CLI, DSL, ACL, API, ASCII, CPU, CSS, DNS, EOF, GUID, HTML, HTTP, HTTPS, ID, IP, JSON, LHS, QPS, RAM, RHS, RPC, SLA, SMTP, SQL, SSH, TCP, TLS, TTL, UDP, UI, UID, UUID, URI, URL, UTF8, VM, XML, XMPP, XSRF, XSS
Style/FileName:
Exclude:
- 'Gemfile'

# Offense count: 1
# Cop supports --auto-correct.
# Configuration parameters: PreferredDelimiters.
Style/PercentLiteralDelimiters:
Exclude:
- 'config/spring.rb'
21 changes: 12 additions & 9 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,16 @@ gem 'sqlite3'
# Use Puma as the app server
gem 'puma', '~> 3.7'
# Use SCSS for stylesheets
gem 'sass-rails', '~> 5.0'
# gem 'sass-rails', '~> 5.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just delete instead of comment out?

Copy link
Contributor Author

@ndushay ndushay Aug 25, 2017

Choose a reason for hiding this comment

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

Excellent question! All of this stuff was added by the rails generator, including pinning to versions. I was feeling tentative, so I commented stuff out. I would be happy to remove cruft if we prefer -- we can always get it back from git history or another rails app if we need it. What do you think?

See also #30 which is about removing directories and the like for unused stuff -- are we comfy with removing all this stuff?

Copy link
Contributor

Choose a reason for hiding this comment

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

My opinion on this one is that we might as well remove this stuff from the Gemfile. These are fairly small changes that would be easy to reinstate, even just by - as you say - running the rails generator again to figure out what we're missing.

I'm less certain about #30 - I believe that removing some of those directories can come back to bite us later and it might be worth just leaving them in there?

# Use Uglifier as compressor for JavaScript assets
gem 'uglifier', '>= 1.3.0'
# gem 'uglifier', '>= 1.3.0'
# See https://github.com/rails/execjs#readme for more supported runtimes
# gem 'therubyracer', platforms: :ruby

# Use CoffeeScript for .coffee assets and views
gem 'coffee-rails', '~> 4.2'
# gem 'coffee-rails', '~> 4.2'
# Turbolinks makes navigating your web application faster. Read more: https://github.com/turbolinks/turbolinks
gem 'turbolinks', '~> 5'
# gem 'turbolinks', '~> 5'
# Build JSON APIs with ease. Read more: https://github.com/rails/jbuilder
gem 'jbuilder', '~> 2.5'
# Use Redis adapter to run Action Cable in production
Expand All @@ -37,25 +37,28 @@ gem 'jbuilder', '~> 2.5'

group :development, :test do
# Call 'byebug' anywhere in the code to stop execution and get a debugger console
gem 'byebug', platforms: [:mri, :mingw, :x64_mingw]
gem 'byebug', platforms: [:mri]
# Call 'binding.pry' anywhere in the code to stop execution and get a pry console
gem 'pry-byebug', require: false
# Adds support for Capybara system testing and selenium driver
gem 'capybara', '~> 2.13'
gem 'selenium-webdriver'
# gem 'capybara', '~> 2.13'
# gem 'selenium-webdriver'
gem 'rspec-rails', '~> 3.6'
gem 'rails-controller-testing'
gem 'coveralls'
# Ruby static code analyzer http://rubocop.readthedocs.io/en/latest/
gem 'rubocop', '~> 0.49.1', require: false # avoid code churn due to rubocop changes
gem 'rubocop-rspec'
end

group :development do
# Access an IRB console on exception pages or by using <%= console %> anywhere in the code.
gem 'web-console', '>= 3.3.0'
# gem 'web-console', '>= 3.3.0'
gem 'listen', '>= 3.0.5', '< 3.2'
# Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
gem 'spring'
gem 'spring-watcher-listen', '~> 2.0.0'
end

# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]
# gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]
78 changes: 20 additions & 58 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,11 @@ GEM
i18n (~> 0.7)
minitest (~> 5.1)
tzinfo (~> 1.1)
addressable (2.5.1)
public_suffix (~> 2.0, >= 2.0.2)
arel (8.0.0)
bindex (0.5.0)
ast (2.3.0)
builder (3.2.3)
byebug (9.0.6)
capybara (2.15.1)
addressable
mini_mime (>= 0.1.3)
nokogiri (>= 1.3.3)
rack (>= 1.0.0)
rack-test (>= 0.5.4)
xpath (~> 2.0)
childprocess (0.7.1)
ffi (~> 1.0, >= 1.0.11)
coderay (1.1.1)
coffee-rails (4.2.2)
coffee-script (>= 2.2.0)
railties (>= 4.0.0)
coffee-script (2.4.1)
coffee-script-source
execjs
coffee-script-source (1.12.2)
concurrent-ruby (1.0.5)
confstruct (1.0.2)
hashie (~> 3.3)
Expand All @@ -75,7 +57,6 @@ GEM
domain_name (0.5.20170404)
unf (>= 0.0.5, < 1.0.0)
erubi (1.6.1)
execjs (2.7.0)
ffi (1.9.18)
globalid (0.4.0)
activesupport (>= 4.2.0)
Expand All @@ -99,7 +80,6 @@ GEM
mime-types (3.1)
mime-types-data (~> 3.2015)
mime-types-data (3.2016.0521)
mini_mime (0.1.4)
mini_portile2 (2.2.0)
minitest (5.10.3)
moab-versioning (2.0.0)
Expand All @@ -115,14 +95,17 @@ GEM
mini_portile2 (~> 2.2.0)
nokogiri-happymapper (0.5.9)
nokogiri (~> 1.5)
parallel (1.11.2)
parser (2.4.0.0)
ast (~> 2.2)
powerpack (0.1.1)
pry (0.10.4)
coderay (~> 1.1.0)
method_source (~> 0.8.1)
slop (~> 3.4)
pry-byebug (3.4.2)
byebug (~> 9.0)
pry (~> 0.10)
public_suffix (2.0.5)
puma (3.9.1)
rack (2.0.3)
rack-test (0.6.3)
Expand Down Expand Up @@ -154,6 +137,8 @@ GEM
method_source
rake (>= 0.8.7)
thor (>= 0.18.1, < 2.0)
rainbow (2.2.2)
rake
rake (12.0.0)
rb-fsevent (0.10.2)
rb-inotify (0.9.10)
Expand All @@ -179,22 +164,17 @@ GEM
rspec-mocks (~> 3.6.0)
rspec-support (~> 3.6.0)
rspec-support (3.6.0)
rubocop (0.49.1)
parallel (~> 1.10)
parser (>= 2.3.3.1, < 3.0)
powerpack (~> 0.1)
rainbow (>= 1.99.1, < 3.0)
ruby-progressbar (~> 1.7)
unicode-display_width (~> 1.0, >= 1.0.1)
rubocop-rspec (1.15.1)
rubocop (>= 0.42.0)
ruby-progressbar (1.8.1)
ruby_dep (1.5.0)
rubyzip (1.2.1)
sass (3.5.1)
sass-listen (~> 4.0.0)
sass-listen (4.0.0)
rb-fsevent (~> 0.9, >= 0.9.4)
rb-inotify (~> 0.9, >= 0.9.7)
sass-rails (5.0.6)
railties (>= 4.0.0, < 6)
sass (~> 3.1)
sprockets (>= 2.8, < 4.0)
sprockets-rails (>= 2.0, < 4.0)
tilt (>= 1.1, < 3)
selenium-webdriver (3.5.1)
childprocess (~> 0.5)
rubyzip (~> 1.0)
simplecov (0.15.0)
docile (~> 1.1.0)
json (>= 1.8, < 3)
Expand All @@ -219,36 +199,22 @@ GEM
tins (~> 1.0)
thor (0.20.0)
thread_safe (0.3.6)
tilt (2.0.8)
tins (1.15.0)
turbolinks (5.0.1)
turbolinks-source (~> 5)
turbolinks-source (5.0.3)
tzinfo (1.2.3)
thread_safe (~> 0.1)
uglifier (3.2.0)
execjs (>= 0.3.0, < 3)
unf (0.1.4)
unf_ext
unf_ext (0.0.7.4)
web-console (3.5.1)
actionview (>= 5.0)
activemodel (>= 5.0)
bindex (>= 0.4.0)
railties (>= 5.0)
unicode-display_width (1.3.0)
websocket-driver (0.6.5)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.2)
xpath (2.1.0)
nokogiri (~> 1.3)

PLATFORMS
ruby

DEPENDENCIES
byebug
capybara (~> 2.13)
coffee-rails (~> 4.2)
coveralls
jbuilder (~> 2.5)
listen (>= 3.0.5, < 3.2)
Expand All @@ -258,15 +224,11 @@ DEPENDENCIES
rails (~> 5.1.3)
rails-controller-testing
rspec-rails (~> 3.6)
sass-rails (~> 5.0)
selenium-webdriver
rubocop (~> 0.49.1)
rubocop-rspec
spring
spring-watcher-listen (~> 2.0.0)
sqlite3
turbolinks (~> 5)
tzinfo-data
uglifier (>= 1.3.0)
web-console (>= 3.3.0)

BUNDLED WITH
1.15.3
5 changes: 5 additions & 0 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@
require_relative 'config/application'

Rails.application.load_tasks

require 'rubocop/rake_task'
RuboCop::RakeTask.new

task default: [:spec, :rubocop]
5 changes: 3 additions & 2 deletions app/controllers/moab_storage_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ def show
private

def druids_from_storage_root
druids ||= begin
@druids ||= begin
@storage_root ||= "#{Moab::Config.storage_roots}/#{Moab::Config.storage_trunk}"
Dir.glob("#{@storage_root}/**/[a-z][a-z]*[0-9]").map {|d| d.split("/").last}
Dir.glob("#{@storage_root}/**/[a-z][a-z]*[0-9]").map { |d| d.split("/").last }
end
@druids
end

end
2 changes: 1 addition & 1 deletion config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ class Application < Rails::Application
require 'moab'
Moab::Config.configure do
# FIXME: put this hardcoded dir in settings.yml file (github issue #21)
storage_roots File.join(File.dirname(__FILE__), '..', 'spec','fixtures')
storage_roots File.join(File.dirname(__FILE__), '..', 'spec', 'fixtures')
storage_trunk 'moab_storage_root'
end
1 change: 1 addition & 0 deletions spec/controllers/moab_storage_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@

describe "GET #show" do
let(:fixture_druid) { 'ct764fs4485' }

it "returns http success status code" do
get :show, params: { id: fixture_druid }
expect(response).to have_http_status(:success)
Expand Down