From b3170396699c927e609c81f6c19e715218e9480d Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Wed, 16 Aug 2017 16:13:56 +0200 Subject: [PATCH 1/6] RuboCop lint rules, linting, TODO config added Signed-off-by: Olle Jonsson --- .rubocop.yml | 21 +++++++++++++ .rubocop_todo.yml | 52 +++++++++++++++++++++++++++++++ Gemfile | 2 +- Rakefile | 5 +-- lib/mixlib/log.rb | 27 +++++++++------- lib/mixlib/log/formatter.rb | 4 +-- lib/mixlib/log/version.rb | 2 +- mixlib-log.gemspec | 2 +- spec/mixlib/log/formatter_spec.rb | 3 +- spec/mixlib/log_spec.rb | 33 ++++++++++---------- spec/spec_helper.rb | 2 +- 11 files changed, 114 insertions(+), 39 deletions(-) create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 0000000..3dc3c28 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,21 @@ +inherit_from: .rubocop_todo.yml + +AllCops: + DisplayCopNames: true + TargetRubyVersion: 2.2 + Exclude: + - 'features/**/*' +Metrics/LineLength: + Max: 120 +Metrics/BlockLength: + Exclude: + - 'spec/**/*' +Style/Documentation: + Enabled: false +Style/FrozenStringLiteralComment: + Enabled: false +# Use double quotes everywhere by default. +Style/StringLiterals: + EnforcedStyle: double_quotes + + diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 0000000..08866cb --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,52 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2017-08-16 10:25:30 +0200 using RuboCop version 0.47.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: 1 +# Configuration parameters: AllowSafeAssignment. +Lint/AssignmentInCondition: + Exclude: + - 'lib/mixlib/log.rb' + +# Offense count: 2 +# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. +# URISchemes: http, https +Metrics/LineLength: + Max: 129 + +# Offense count: 1 +# Configuration parameters: CountComments. +Metrics/MethodLength: + Max: 12 + +# Offense count: 1 +# Configuration parameters: CountComments. +Metrics/ModuleLength: + Max: 109 + +# Offense count: 2 +Style/ClassVars: + Exclude: + - 'lib/mixlib/log/formatter.rb' + +# Offense count: 2 +# Configuration parameters: EnforcedStyle, SupportedStyles. +# SupportedStyles: format, sprintf, percent +Style/FormatString: + Exclude: + - 'lib/mixlib/log/formatter.rb' + +# Offense count: 1 +# Configuration parameters: AllowedVariables. +Style/GlobalVars: + Exclude: + - 'spec/spec_helper.rb' + +# Offense count: 1 +Style/MethodMissing: + Exclude: + - 'lib/mixlib/log.rb' diff --git a/Gemfile b/Gemfile index 13ce910..a8fa97f 100755 --- a/Gemfile +++ b/Gemfile @@ -3,6 +3,6 @@ source "https://rubygems.org" gemspec group :development do - gem "rdoc" gem "bundler" + gem "rdoc" end diff --git a/Rakefile b/Rakefile index 9b15524..4507bdd 100644 --- a/Rakefile +++ b/Rakefile @@ -10,7 +10,7 @@ end task default: [:style, :spec, :features] # For rubygems-test -task :test => :spec +task test: :spec RDoc::Task.new do |rdoc| rdoc.rdoc_dir = "rdoc" @@ -39,5 +39,6 @@ GitHubChangelogGenerator::RakeTask.new :changelog do |config| config.future_release = Mixlib::Log::VERSION config.enhancement_labels = "enhancement,Enhancement,New Feature,Feature".split(",") config.bug_labels = "bug,Bug,Improvement,Upstream Bug".split(",") - config.exclude_labels = "duplicate,question,invalid,wontfix,no_changelog,Exclude From Changelog,Question,Discussion".split(",") + config.exclude_labels = ["duplicate", "question", "invalid", "wontfix", + "no_changelog", "Exclude From Changelog", "Question", "Discussion"] end diff --git a/lib/mixlib/log.rb b/lib/mixlib/log.rb index 7152a90..ccbeded 100644 --- a/lib/mixlib/log.rb +++ b/lib/mixlib/log.rb @@ -22,15 +22,16 @@ module Mixlib module Log - @logger, @loggers = nil - LEVELS = { :debug => Logger::DEBUG, :info => Logger::INFO, :warn => Logger::WARN, :error => Logger::ERROR, :fatal => Logger::FATAL }.freeze + LEVELS = { debug: Logger::DEBUG, info: Logger::INFO, + warn: Logger::WARN, error: Logger::ERROR, fatal: Logger::FATAL }.freeze LEVEL_NAMES = LEVELS.invert.freeze def reset! close! - @logger, @loggers = nil, nil + @logger = nil + @loggers = nil end # An Array of log devices that will be logged to. Defaults to just the default @@ -58,13 +59,12 @@ def use_log_devices(other) if other.respond_to?(:loggers) && other.respond_to?(:logger) @loggers = other.loggers @logger = other.logger - elsif other.kind_of?(Array) + elsif other.is_a?(Array) @loggers = other @logger = other.first else - msg = "#use_log_devices takes a Mixlib::Log object or array of log devices. " << - "You gave: #{other.inspect}" - raise ArgumentError, msg + raise ArgumentError, "#use_log_devices takes a Mixlib::Log object or array of log devices. " \ + "You gave: #{other.inspect}" end @configured = true end @@ -79,7 +79,7 @@ def use_log_devices(other) def init(*opts) reset! @logger = logger_for(*opts) - @logger.formatter = Mixlib::Log::Formatter.new() if @logger.respond_to?(:formatter=) + @logger.formatter = Mixlib::Log::Formatter.new if @logger.respond_to?(:formatter=) @logger.level = Logger::WARN @configured = true @logger @@ -109,7 +109,7 @@ def level(new_level = nil) if new_level.nil? LEVEL_NAMES[logger.level] else - self.level = (new_level) + self.level = new_level end end @@ -143,7 +143,7 @@ def add(severity, message = nil, progname = nil, &block) loggers.each { |l| l.add(severity, message, progname, &block) } end - alias :log :add + alias log add # Passes any other method calls on directly to the underlying Logger object created with init. If # this method gets hit before a call to Mixlib::Logger.init has been made, it will call @@ -185,9 +185,12 @@ def loggers_to_close def close! # try to close all file loggers loggers_to_close.each do |l| - l.close rescue nil + begin + l.close + rescue + nil + end end end - end end diff --git a/lib/mixlib/log/formatter.rb b/lib/mixlib/log/formatter.rb index c8f1efa..e02b65d 100644 --- a/lib/mixlib/log/formatter.rb +++ b/lib/mixlib/log/formatter.rb @@ -29,9 +29,9 @@ def self.show_time=(show = false) # Prints a log message as '[time] severity: message' if Chef::Log::Formatter.show_time == true. # Otherwise, doesn't print the time. - def call(severity, time, progname, msg) + def call(severity, time, _progname, msg) if @@show_time - sprintf("[%s] %s: %s\n", time.iso8601(), severity, msg2str(msg)) + sprintf("[%s] %s: %s\n", time.iso8601, severity, msg2str(msg)) else sprintf("%s: %s\n", severity, msg2str(msg)) end diff --git a/lib/mixlib/log/version.rb b/lib/mixlib/log/version.rb index 814320f..f7ea573 100644 --- a/lib/mixlib/log/version.rb +++ b/lib/mixlib/log/version.rb @@ -1,5 +1,5 @@ module Mixlib module Log - VERSION = "1.7.1" + VERSION = "1.7.1".freeze end end diff --git a/mixlib-log.gemspec b/mixlib-log.gemspec index 82ea5b3..035ad54 100644 --- a/mixlib-log.gemspec +++ b/mixlib-log.gemspec @@ -1,4 +1,4 @@ -$:.unshift File.expand_path("../lib", __FILE__) +$LOAD_PATH.unshift File.expand_path("../lib", __FILE__) require "mixlib/log/version" Gem::Specification.new do |gem| diff --git a/spec/mixlib/log/formatter_spec.rb b/spec/mixlib/log/formatter_spec.rb index f4d664b..f15e03e 100644 --- a/spec/mixlib/log/formatter_spec.rb +++ b/spec/mixlib/log/formatter_spec.rb @@ -34,7 +34,7 @@ end it "should format random objects via inspect with msg2str(Object)" do - expect(@formatter.msg2str([ "black thought", "?uestlove" ])).to eq('["black thought", "?uestlove"]') + expect(@formatter.msg2str(["black thought", "?uestlove"])).to eq('["black thought", "?uestlove"]') end it "should return a formatted string with call" do @@ -47,5 +47,4 @@ Mixlib::Log::Formatter.show_time = false expect(@formatter.call("monkey", Time.new, "test", "mos def")).to eq("monkey: mos def\n") end - end diff --git a/spec/mixlib/log_spec.rb b/spec/mixlib/log_spec.rb index d780b0d..8ff231a 100644 --- a/spec/mixlib/log_spec.rb +++ b/spec/mixlib/log_spec.rb @@ -38,7 +38,6 @@ def #{method_name}(message) end describe Mixlib::Log do - # Since we are testing class behaviour for an instance variable # that gets set once, we need to reset it prior to each example [cb] before(:each) do @@ -69,7 +68,8 @@ def #{method_name}(message) end it "should re-initialize the logger if init is called again" do - first_logdev, second_logdev = StringIO.new, StringIO.new + first_logdev = StringIO.new + second_logdev = StringIO.new Logit.init(first_logdev) Logit.fatal "FIRST" expect(first_logdev.string).to match(/FIRST/) @@ -86,11 +86,11 @@ def #{method_name}(message) it "should set the log level using the binding form, with :debug, :info, :warn, :error, or :fatal" do levels = { - :debug => Logger::DEBUG, - :info => Logger::INFO, - :warn => Logger::WARN, - :error => Logger::ERROR, - :fatal => Logger::FATAL, + debug: Logger::DEBUG, + info: Logger::INFO, + warn: Logger::WARN, + error: Logger::ERROR, + fatal: Logger::FATAL } levels.each do |symbol, constant| Logit.level = symbol @@ -108,11 +108,11 @@ def #{method_name}(message) it "should set the log level using the method form, with :debug, :info, :warn, :error, or :fatal" do levels = { - :debug => Logger::DEBUG, - :info => Logger::INFO, - :warn => Logger::WARN, - :error => Logger::ERROR, - :fatal => Logger::FATAL, + debug: Logger::DEBUG, + info: Logger::INFO, + warn: Logger::WARN, + error: Logger::ERROR, + fatal: Logger::FATAL } levels.each do |symbol, constant| Logit.level(symbol) @@ -121,17 +121,17 @@ def #{method_name}(message) end it "should raise an ArgumentError if you try and set the level to something strange using the binding form" do - expect(lambda { Logit.level = :the_roots }).to raise_error(ArgumentError) + expect(-> { Logit.level = :the_roots }).to raise_error(ArgumentError) end it "should raise an ArgumentError if you try and set the level to something strange using the method form" do - expect(lambda { Logit.level(:the_roots) }).to raise_error(ArgumentError) + expect(-> { Logit.level(:the_roots) }).to raise_error(ArgumentError) end it "should pass other method calls directly to logger" do Logit.level = :debug expect(Logit).to be_debug - expect(lambda { Logit.debug("Gimme some sugar!") }).to_not raise_error + expect(-> { Logit.debug("Gimme some sugar!") }).to_not raise_error end it "should pass add method calls directly to logger" do @@ -139,7 +139,7 @@ def #{method_name}(message) Logit.init(logdev) Logit.level = :debug expect(Logit).to be_debug - expect(lambda { Logit.add(Logger::DEBUG, "Gimme some sugar!") }).to_not raise_error + expect(-> { Logit.add(Logger::DEBUG, "Gimme some sugar!") }).to_not raise_error expect(logdev.string).to match(/Gimme some sugar/) end @@ -186,5 +186,4 @@ def #{method_name}(message) end expect(opened_files_count_after).to eq(opened_files_count_before + 1) end - end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3461464..1cb45b1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -18,7 +18,7 @@ # $TESTING = true -$:.push File.join(File.dirname(__FILE__), "..", "lib") +$LOAD_PATH.push File.join(File.dirname(__FILE__), "..", "lib") require "rspec" require "mixlib/log" From f237a8ce570985d61100983671985fb1b5da3f64 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 17 Aug 2017 10:55:45 +0200 Subject: [PATCH 2/6] RuboCop: 0.49.1, which uses chefstyle 0.6.0 Signed-off-by: Olle Jonsson --- Gemfile | 1 + Rakefile | 2 +- lib/mixlib/log.rb | 4 ++-- spec/mixlib/log_spec.rb | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index a8fa97f..bda5e3f 100755 --- a/Gemfile +++ b/Gemfile @@ -5,4 +5,5 @@ gemspec group :development do gem "bundler" gem "rdoc" + gem "rubocop", "~> 0.49.1" end diff --git a/Rakefile b/Rakefile index 4507bdd..40c218b 100644 --- a/Rakefile +++ b/Rakefile @@ -7,7 +7,7 @@ RSpec::Core::RakeTask.new(:spec) do |spec| spec.pattern = "spec/**/*_spec.rb" end -task default: [:style, :spec, :features] +task default: %i[style spec features] # For rubygems-test task test: :spec diff --git a/lib/mixlib/log.rb b/lib/mixlib/log.rb index ccbeded..fc4a332 100644 --- a/lib/mixlib/log.rb +++ b/lib/mixlib/log.rb @@ -115,7 +115,7 @@ def level(new_level = nil) # Define the standard logger methods on this class programmatically. # No need to incur method_missing overhead on every log call. - [:debug, :info, :warn, :error, :fatal].each do |method_name| + %i[debug info warn error fatal].each do |method_name| class_eval(<<-METHOD_DEFN, __FILE__, __LINE__) def #{method_name}(msg=nil, &block) loggers.each {|l| l.#{method_name}(msg, &block) } @@ -127,7 +127,7 @@ def #{method_name}(msg=nil, &block) # Note that we *only* query the default logger (@logger) and not any other # loggers that may have been added, even though it is possible to configure # two (or more) loggers at different log levels. - [:debug?, :info?, :warn?, :error?, :fatal?].each do |method_name| + %i[debug? info? warn? error? fatal?].each do |method_name| class_eval(<<-METHOD_DEFN, __FILE__, __LINE__) def #{method_name} logger.#{method_name} diff --git a/spec/mixlib/log_spec.rb b/spec/mixlib/log_spec.rb index 8ff231a..1805da1 100644 --- a/spec/mixlib/log_spec.rb +++ b/spec/mixlib/log_spec.rb @@ -28,7 +28,7 @@ def initialize @messages = "" end - [:debug, :info, :warn, :error, :fatal].each do |method_name| + %i[debug info warn error fatal].each do |method_name| class_eval(<<-E) def #{method_name}(message) @messages << message From 110ed94f3baa8c5f92a10fa3c9720b24ac0c3392 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 17 Aug 2017 10:58:26 +0200 Subject: [PATCH 3/6] Mark chefstyle as 0.6 or higher in gemspec Signed-off-by: Olle Jonsson --- mixlib-log.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mixlib-log.gemspec b/mixlib-log.gemspec index 035ad54..de4654a 100644 --- a/mixlib-log.gemspec +++ b/mixlib-log.gemspec @@ -16,7 +16,7 @@ Gem::Specification.new do |gem| gem.required_ruby_version = ">= 2.2" gem.add_development_dependency "rake" gem.add_development_dependency "rspec", "~> 3.4" - gem.add_development_dependency "chefstyle" + gem.add_development_dependency "chefstyle", ">= 0.6" gem.add_development_dependency "cucumber" gem.add_development_dependency "github_changelog_generator", ">= 1.11.3" end From ca3b7310eb6bec858bf8c7010b3374c71acf2bd7 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 17 Aug 2017 11:16:46 +0200 Subject: [PATCH 4/6] chefstyle: Actually use chefstyle Signed-off-by: Olle Jonsson --- Rakefile | 2 +- lib/mixlib/log.rb | 4 ++-- spec/mixlib/log_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Rakefile b/Rakefile index 40c218b..9e18055 100644 --- a/Rakefile +++ b/Rakefile @@ -7,7 +7,7 @@ RSpec::Core::RakeTask.new(:spec) do |spec| spec.pattern = "spec/**/*_spec.rb" end -task default: %i[style spec features] +task default: %i{style spec features} # For rubygems-test task test: :spec diff --git a/lib/mixlib/log.rb b/lib/mixlib/log.rb index fc4a332..fdc4871 100644 --- a/lib/mixlib/log.rb +++ b/lib/mixlib/log.rb @@ -115,7 +115,7 @@ def level(new_level = nil) # Define the standard logger methods on this class programmatically. # No need to incur method_missing overhead on every log call. - %i[debug info warn error fatal].each do |method_name| + %i{debug info warn error fatal}.each do |method_name| class_eval(<<-METHOD_DEFN, __FILE__, __LINE__) def #{method_name}(msg=nil, &block) loggers.each {|l| l.#{method_name}(msg, &block) } @@ -127,7 +127,7 @@ def #{method_name}(msg=nil, &block) # Note that we *only* query the default logger (@logger) and not any other # loggers that may have been added, even though it is possible to configure # two (or more) loggers at different log levels. - %i[debug? info? warn? error? fatal?].each do |method_name| + %i{debug? info? warn? error? fatal?}.each do |method_name| class_eval(<<-METHOD_DEFN, __FILE__, __LINE__) def #{method_name} logger.#{method_name} diff --git a/spec/mixlib/log_spec.rb b/spec/mixlib/log_spec.rb index 1805da1..2a1923d 100644 --- a/spec/mixlib/log_spec.rb +++ b/spec/mixlib/log_spec.rb @@ -28,7 +28,7 @@ def initialize @messages = "" end - %i[debug info warn error fatal].each do |method_name| + %i{debug info warn error fatal}.each do |method_name| class_eval(<<-E) def #{method_name}(message) @messages << message @@ -90,7 +90,7 @@ def #{method_name}(message) info: Logger::INFO, warn: Logger::WARN, error: Logger::ERROR, - fatal: Logger::FATAL + fatal: Logger::FATAL, } levels.each do |symbol, constant| Logit.level = symbol @@ -112,7 +112,7 @@ def #{method_name}(message) info: Logger::INFO, warn: Logger::WARN, error: Logger::ERROR, - fatal: Logger::FATAL + fatal: Logger::FATAL, } levels.each do |symbol, constant| Logit.level(symbol) From acffcb4f056209c0ed7ff1ce535aac4cbb0720f0 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 17 Aug 2017 12:53:50 +0200 Subject: [PATCH 5/6] Updated TODO with RuboCop 0.49.1 Signed-off-by: Olle Jonsson --- .rubocop_todo.yml | 47 +---------------------------------------------- 1 file changed, 1 insertion(+), 46 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 08866cb..e326317 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,52 +1,7 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2017-08-16 10:25:30 +0200 using RuboCop version 0.47.1. +# on 2017-08-17 12:53:11 +0200 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: 1 -# Configuration parameters: AllowSafeAssignment. -Lint/AssignmentInCondition: - Exclude: - - 'lib/mixlib/log.rb' - -# Offense count: 2 -# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 129 - -# Offense count: 1 -# Configuration parameters: CountComments. -Metrics/MethodLength: - Max: 12 - -# Offense count: 1 -# Configuration parameters: CountComments. -Metrics/ModuleLength: - Max: 109 - -# Offense count: 2 -Style/ClassVars: - Exclude: - - 'lib/mixlib/log/formatter.rb' - -# Offense count: 2 -# Configuration parameters: EnforcedStyle, SupportedStyles. -# SupportedStyles: format, sprintf, percent -Style/FormatString: - Exclude: - - 'lib/mixlib/log/formatter.rb' - -# Offense count: 1 -# Configuration parameters: AllowedVariables. -Style/GlobalVars: - Exclude: - - 'spec/spec_helper.rb' - -# Offense count: 1 -Style/MethodMissing: - Exclude: - - 'lib/mixlib/log.rb' From 9e6d817e0a35d47790ddf5c922c77d3ebb813438 Mon Sep 17 00:00:00 2001 From: Olle Jonsson Date: Thu, 17 Aug 2017 13:35:54 +0200 Subject: [PATCH 6/6] RuboCop: ignore vendor/ Signed-off-by: Olle Jonsson --- .rubocop.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop.yml b/.rubocop.yml index 3dc3c28..e543aa9 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -5,6 +5,7 @@ AllCops: TargetRubyVersion: 2.2 Exclude: - 'features/**/*' + - 'vendor/**/*' Metrics/LineLength: Max: 120 Metrics/BlockLength: