Skip to content

Commit

Permalink
Update rubocop exceptions, and solve the each_key/each_value problem
Browse files Browse the repository at this point in the history
The rule was triggering on non-hash `Config::Options` objects, because
they are very `Hash`-like.  Easy solution is to convert the part that we
care about to a regular hash, when we are going to iterate in a way that
only wants keys or only wants values.

This commit includes removing other exceptions that are no longer applicable!
  • Loading branch information
atz committed Jun 5, 2018
1 parent 88386c9 commit 50f661d
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 35 deletions.
28 changes: 7 additions & 21 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ Metrics/AbcSize:
- 'app/services/checksum_validator.rb' # methods still readable; shameless green
- 'app/services/preserved_object_handler.rb' # methods still readable; shameless green
- 'app/services/audit_results.rb' # method still readable
- 'lib/audit/catalog_to_moab.rb' # .check_version_all_dirs is decidedly readable
- 'lib/audit/checksum.rb' # .validate_disk_all_dirs is decidedly readable
- 'lib/audit/moab_to_catalog.rb' # .xxx_for_all_storage_roots is decidedly readable
- 'app/lib/audit/catalog_to_moab.rb' # .check_version_all_dirs is decidedly readable
- 'app/lib/audit/checksum.rb' # .validate_disk_all_dirs is decidedly readable
- 'app/lib/audit/moab_to_catalog.rb' # .xxx_for_all_storage_roots is decidedly readable
- 'app/controllers/catalog_controller.rb' # #create method is readable

Metrics/BlockLength:
Expand All @@ -82,13 +82,13 @@ Metrics/ClassLength:
- 'app/services/checksum_validator.rb'
- 'app/services/preserved_object_handler.rb'
- 'app/services/audit_results.rb'
- 'lib/audit/catalog_to_moab.rb'
- 'app/lib/audit/catalog_to_moab.rb'

Metrics/CyclomaticComplexity:
Exclude:
- 'app/services/preserved_object_handler.rb' # update_online_version (shameless green)
- 'app/services/audit_results.rb' # logger_severity_level is simply a case statement
- 'lib/audit/catalog_to_moab.rb' # check_catalog_version (shameless green)
- 'app/lib/audit/catalog_to_moab.rb' # check_catalog_version (shameless green)

# because this isn't 1994
Metrics/LineLength:
Expand Down Expand Up @@ -116,12 +116,12 @@ Metrics/MethodLength:
Max: 25
Exclude:
- 'app/services/preserved_object_handler.rb' # check_existence shameless green
- 'lib/audit/catalog_to_moab.rb' # check_catalog_version shameless green
- 'app/lib/audit/catalog_to_moab.rb' # check_catalog_version shameless green

Metrics/PerceivedComplexity:
Exclude:
- 'app/services/preserved_object_handler.rb' # update_online_version shameless green
- 'lib/audit/catalog_to_moab.rb' # check_catalog_version shameless green
- 'app/lib/audit/catalog_to_moab.rb' # check_catalog_version shameless green

# --- Naming ---

Expand All @@ -134,25 +134,11 @@ Naming/FileName:
- 'Capfile'
- 'Gemfile'

# --- Performance ---

Performance/HashEachMethods:
Exclude:
- lib/audit/catalog_to_moab.rb # it's a tiny config object; code is clearer as is
- lib/audit/checksum.rb # it's a tiny config object; code is clearer as is
- lib/audit/moab_to_catalog.rb # it's a tiny config object; code is clearer as is

# --- Rails ---

Rails/HasAndBelongsToMany:
Enabled: false

Rails/Output:
Exclude:
- 'lib/audit/catalog_to_moab.rb' # use puts statements for following progress of long job
- 'lib/audit/checksum.rb' # use puts statements for following progress of long job
- 'lib/audit/moab_to_catalog.rb' # use puts statements for following progress of long job

# --- RSpec ---

RSpec/BeforeAfterAll:
Expand Down
2 changes: 1 addition & 1 deletion app/lib/audit/catalog_to_moab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def self.check_version_on_dir_profiled(last_checked_b4_date, storage_dir)

def self.check_version_all_dirs(last_checked_b4_date)
logger.info "#{Time.now.utc.iso8601} C2M check_version_all_dirs starting"
HostSettings.storage_roots.each do |_strg_root_name, strg_root_location|
HostSettings.storage_roots.to_h.each_value do |strg_root_location|
check_version_on_dir(last_checked_b4_date, "#{strg_root_location}/#{Settings.moab.storage_trunk}")
end
ensure
Expand Down
2 changes: 1 addition & 1 deletion app/lib/audit/checksum.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def self.validate_disk_profiled(endpoint_name)

def self.validate_disk_all_endpoints
logger.info "#{Time.now.utc.iso8601} CV validate_disk_all_endpoints starting"
HostSettings.storage_roots.each do |strg_root_name, _strg_root_location|
HostSettings.storage_roots.to_h.each_key do |strg_root_name|
validate_disk(strg_root_name)
end
ensure
Expand Down
4 changes: 2 additions & 2 deletions app/lib/audit/moab_to_catalog.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def self.seed_catalog_for_dir(storage_dir)
# Shameless green. In order to run several seed "jobs" in parallel, we would have to refactor.
def self.seed_catalog_for_all_storage_roots
logger.info "#{Time.now.utc.iso8601} Seeding for all storage roots starting"
HostSettings.storage_roots.each do |_strg_root_name, strg_root_location|
HostSettings.storage_roots.to_h.each_value do |strg_root_location|
seed_catalog_for_dir("#{strg_root_location}/#{Settings.moab.storage_trunk}")
end
ensure
Expand All @@ -74,7 +74,7 @@ def self.seed_catalog_for_all_storage_roots_profiled
# Shameless green. Code duplication with seed_catalog_for_all_storage_roots
def self.check_existence_for_all_storage_roots
logger.info "#{Time.now.utc.iso8601} M2C check_existence for all storage roots starting'"
HostSettings.storage_roots.each do |_strg_root_name, strg_root_location|
HostSettings.storage_roots.to_h.each_value do |strg_root_location|
check_existence_for_dir("#{strg_root_location}/#{Settings.moab.storage_trunk}")
end
ensure
Expand Down
2 changes: 1 addition & 1 deletion spec/lib/audit/checksum_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@
end

it 'calls validate_disk with the right arguments' do
HostSettings.storage_roots.each_key do |storage_name|
HostSettings.storage_roots.to_h.each_key do |storage_name|
expect(described_class).to receive(:validate_disk).with(
storage_name
)
Expand Down
8 changes: 4 additions & 4 deletions spec/lib/audit/moab_to_catalog_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
end

it 'calls check_existence_for_dir with the right arguments' do
HostSettings.storage_roots.each do |storage_root|
expect(described_class).to receive(:check_existence_for_dir).with("#{storage_root[1]}/#{Settings.moab.storage_trunk}")
HostSettings.storage_roots.to_h.each_value do |path|
expect(described_class).to receive(:check_existence_for_dir).with("#{path}/#{Settings.moab.storage_trunk}")
end
subject
end
Expand Down Expand Up @@ -49,8 +49,8 @@
end

it 'calls seed_catalog_for_dir with the right arguments' do
HostSettings.storage_roots.each do |storage_root|
expect(described_class).to receive(:seed_catalog_for_dir).with("#{storage_root[1]}/#{Settings.moab.storage_trunk}")
HostSettings.storage_roots.to_h.each_value do |path|
expect(described_class).to receive(:seed_catalog_for_dir).with("#{path}/#{Settings.moab.storage_trunk}")
end
subject
end
Expand Down
7 changes: 2 additions & 5 deletions spec/load_fixtures_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
end
after do
# TODO: danger - if there are objects in the test db we want to keep
HostSettings.storage_roots.each_value do |storage_root|
HostSettings.storage_roots.to_h.each_value do |storage_root|
storage_dir = File.join(storage_root, Settings.moab.storage_trunk)
PreservedCopy.where(endpoint_id: Endpoint.find_by(storage_location: storage_dir).id).each do |pc|
po_id = pc.preserved_object_id
Expand Down Expand Up @@ -48,12 +48,9 @@ def load_fixture_moabs
def setup
@moab_storage_dirs = []
@storage_dir_to_endpoint_id = {}
# FIXME: I couldn't get .each_value to work ... try again?
# rubocop:disable Performance/HashEachMethods
HostSettings.storage_roots.each do |_name, storage_root|
HostSettings.storage_roots.to_h.each_value do |storage_root|
storage_dir = File.join(storage_root, Settings.moab.storage_trunk)
@moab_storage_dirs << storage_dir
@storage_dir_to_endpoint_id[storage_dir] = Endpoint.find_by(storage_location: storage_dir).id
end
# rubocop:enable Performance/HashEachMethods
end

0 comments on commit 50f661d

Please sign in to comment.