From 81e283b28cdd91d259e3b60687aee7ea66e9d05d Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Fri, 9 Jul 2010 18:06:06 -0700 Subject: Code smell: Line modifiers are preferred to one-line blocks. * Replaced 6 occurances of (while .*?) *do$ with The do is unneeded in the block header form and causes problems with the block-to-one-line transformation. 3 Examples: The code: while line = f.gets do becomes: while line = f.gets The code: while line = shadow.gets do becomes: while line = shadow.gets The code: while wrapper = zeros.pop do becomes: while wrapper = zeros.pop * Replaced 19 occurances of ((if|unless) .*?) *then$ with The then is unneeded in the block header form and causes problems with the block-to-one-line transformation. 3 Examples: The code: if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } then becomes: if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) } The code: unless defined?(@spec_command) then becomes: unless defined?(@spec_command) The code: if c == ?\n then becomes: if c == ?\n * Replaced 758 occurances of ((?:if|unless|while|until) .*) (.*) end with The one-line form is preferable provided: * The condition is not used to assign a variable * The body line is not already modified * The resulting line is not too long 3 Examples: The code: if Puppet.features.libshadow? has_feature :manages_passwords end becomes: has_feature :manages_passwords if Puppet.features.libshadow? The code: unless (defined?(@current_pool) and @current_pool) @current_pool = process_zpool_data(get_pool_data) end becomes: @current_pool = process_zpool_data(get_pool_data) unless (defined?(@current_pool) and @current_pool) The code: if Puppet[:trace] puts detail.backtrace end becomes: puts detail.backtrace if Puppet[:trace] --- lib/puppet/network/handler/ca.rb | 8 ++---- lib/puppet/network/handler/filebucket.rb | 4 +-- lib/puppet/network/handler/fileserver.rb | 44 ++++++++------------------------ lib/puppet/network/handler/master.rb | 4 +-- lib/puppet/network/handler/report.rb | 12 +++------ 5 files changed, 18 insertions(+), 54 deletions(-) (limited to 'lib/puppet/network/handler') diff --git a/lib/puppet/network/handler/ca.rb b/lib/puppet/network/handler/ca.rb index 8bafb5f9b..c72171d5d 100644 --- a/lib/puppet/network/handler/ca.rb +++ b/lib/puppet/network/handler/ca.rb @@ -60,9 +60,7 @@ class Puppet::Network::Handler def initialize(hash = {}) Puppet.settings.use(:main, :ssl, :ca) - if hash.include? :autosign - @autosign = hash[:autosign] - end + @autosign = hash[:autosign] if hash.include? :autosign @ca = Puppet::SSLCertificates::CA.new(hash) end @@ -109,9 +107,7 @@ class Puppet::Network::Handler return [cert.to_pem, cacert.to_pem] elsif @ca if self.autosign?(hostname) or client.nil? - if client.nil? - Puppet.info "Signing certificate for CA server" - end + Puppet.info "Signing certificate for CA server" if client.nil? # okay, we don't have a signed cert # if we're a CA and autosign is turned on, then go ahead and sign # the csr and return the results diff --git a/lib/puppet/network/handler/filebucket.rb b/lib/puppet/network/handler/filebucket.rb index bea1c85f5..13fee1661 100755 --- a/lib/puppet/network/handler/filebucket.rb +++ b/lib/puppet/network/handler/filebucket.rb @@ -26,9 +26,7 @@ class Puppet::Network::Handler # :nodoc: # Accept a file from a client and store it by md5 sum, returning # the sum. def addfile(contents, path, client = nil, clientip = nil) - if client - contents = Base64.decode64(contents) - end + contents = Base64.decode64(contents) if client bucket = Puppet::FileBucket::File.new(contents) return bucket.save end diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb index 756e74909..cb54ca4e4 100755 --- a/lib/puppet/network/handler/fileserver.rb +++ b/lib/puppet/network/handler/fileserver.rb @@ -98,22 +98,16 @@ class Puppet::Network::Handler @local = false end - if hash[:Config] == false - @noreadconfig = true - end + @noreadconfig = true if hash[:Config] == false @passed_configuration_path = hash[:Config] if hash.include?(:Mount) @passedconfig = true - unless hash[:Mount].is_a?(Hash) - raise Puppet::DevError, "Invalid mount hash #{hash[:Mount].inspect}" - end + raise Puppet::DevError, "Invalid mount hash #{hash[:Mount].inspect}" unless hash[:Mount].is_a?(Hash) hash[:Mount].each { |dir, name| - if FileTest.exists?(dir) - self.mount(dir, name) - end + self.mount(dir, name) if FileTest.exists?(dir) } self.mount(nil, MODULES) self.mount(nil, PLUGINS) @@ -179,9 +173,7 @@ class Puppet::Network::Handler mount, path = convert(url, client, clientip) - if client - mount.info "Sending #{url} to #{client}" - end + mount.info "Sending #{url} to #{client}" if client unless mount.path_exists?(path, client) mount.debug "#{mount} reported that #{path} does not exist" @@ -264,9 +256,7 @@ class Puppet::Network::Handler return unless configuration - if check and ! @configuration.changed? - return - end + return if check and ! @configuration.changed? newmounts = {} begin @@ -279,9 +269,7 @@ class Puppet::Network::Handler when /^\s*$/; next # skip blank lines when /\[([-\w]+)\]/ name = $1 - if newmounts.include?(name) - raise FileServerError, "#{newmounts[name]} is already mounted as #{name} in #{@configuration.file}" - end + raise FileServerError, "#{newmounts[name]} is already mounted as #{name} in #{@configuration.file}" if newmounts.include?(name) mount = Mount.new(name) newmounts[name] = mount when /^\s*(\w+)\s+(.+)$/ @@ -379,9 +367,7 @@ class Puppet::Network::Handler # We let the check raise an error, so that it can raise an error # pointing to the specific problem. newmounts.each { |name, mount| - unless mount.valid? - raise FileServerError, "Invalid mount #{name}" - end + raise FileServerError, "Invalid mount #{name}" unless mount.valid? } @mounts = newmounts end @@ -529,9 +515,7 @@ class Puppet::Network::Handler end # This, ah, might be completely redundant - unless obj[:links] == links - obj[:links] = links - end + obj[:links] = links unless obj[:links] == links return obj end @@ -572,15 +556,9 @@ class Puppet::Network::Handler # Mark that we're expandable. @expandable = true else - unless FileTest.exists?(path) - raise FileServerError, "#{path} does not exist" - end - unless FileTest.directory?(path) - raise FileServerError, "#{path} is not a directory" - end - unless FileTest.readable?(path) - raise FileServerError, "#{path} is not readable" - end + raise FileServerError, "#{path} does not exist" unless FileTest.exists?(path) + raise FileServerError, "#{path} is not a directory" unless FileTest.directory?(path) + raise FileServerError, "#{path} is not readable" unless FileTest.readable?(path) @expandable = false end @path = path diff --git a/lib/puppet/network/handler/master.rb b/lib/puppet/network/handler/master.rb index beb6e4c57..d55046b5b 100644 --- a/lib/puppet/network/handler/master.rb +++ b/lib/puppet/network/handler/master.rb @@ -45,9 +45,7 @@ class Puppet::Network::Handler # This is only used by the cfengine module, or if --loadclasses was # specified in +puppet+. - if hash.include?(:Classes) - args[:Classes] = hash[:Classes] - end + args[:Classes] = hash[:Classes] if hash.include?(:Classes) end # Call our various handlers; this handler is getting deprecated. diff --git a/lib/puppet/network/handler/report.rb b/lib/puppet/network/handler/report.rb index 9f885518e..50e2fbfea 100755 --- a/lib/puppet/network/handler/report.rb +++ b/lib/puppet/network/handler/report.rb @@ -24,18 +24,14 @@ class Puppet::Network::Handler # Accept a report from a client. def report(report, client = nil, clientip = nil) # Unescape the report - unless @local - report = CGI.unescape(report) - end + report = CGI.unescape(report) unless @local Puppet.info "Processing reports #{reports().join(", ")} for #{client}" begin process(report) rescue => detail Puppet.err "Could not process report for #{client}: #{detail}" - if Puppet[:trace] - puts detail.backtrace - end + puts detail.backtrace if Puppet[:trace] end end @@ -65,9 +61,7 @@ class Puppet::Network::Handler newrep.extend(mod) newrep.process rescue => detail - if Puppet[:trace] - puts detail.backtrace - end + puts detail.backtrace if Puppet[:trace] Puppet.err "Report #{name} failed: #{detail}" end else -- cgit