<feed xmlns='http://www.w3.org/2005/Atom'>
<title>puppet.git/lib/puppet/util/settings, branch master</title>
<subtitle>Puppet repo</subtitle>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/'/>
<entry>
<title>(#8662) Don't manage internal file permissions on Windows</title>
<updated>2011-08-22T22:07:41+00:00</updated>
<author>
<name>Josh Cooper</name>
<email>josh@puppetlabs.com</email>
</author>
<published>2011-08-22T16:26:33+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=0f207a8ea61205ba6b47e8da429ab3887b3cda4d'/>
<id>0f207a8ea61205ba6b47e8da429ab3887b3cda4d</id>
<content type='text'>
When running as root, puppet will by default manage internal file
permissions for file-related settings. However, ruby does not support
chown/chgrp functionality on Windows, so puppet will fail to run
(puppet apply generates an exception while trying to set the owner,
etc).

This commit disables internal file permissions handling on Windows
until we add support for chown (at least) as part of the larger file
type effort on Windows.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When running as root, puppet will by default manage internal file
permissions for file-related settings. However, ruby does not support
chown/chgrp functionality on Windows, so puppet will fail to run
(puppet apply generates an exception while trying to set the owner,
etc).

This commit disables internal file permissions handling on Windows
until we add support for chown (at least) as part of the larger file
type effort on Windows.
</pre>
</div>
</content>
</entry>
<entry>
<title>Stop trying to make config directories in Windows specs</title>
<updated>2011-08-19T20:52:59+00:00</updated>
<author>
<name>Nick Lewis</name>
<email>nick@puppetlabs.com</email>
</author>
<published>2011-08-10T20:55:39+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=b6ca78cc8d28dd51ce64de42d20f6fb49dfe814a'/>
<id>b6ca78cc8d28dd51ce64de42d20f6fb49dfe814a</id>
<content type='text'>
FileSetting#to_resource had a provision to not attempt to manage files in /dev,
which effectively stopped spec from trying to make their configuration
directories in /dev/null. However, on Windows, the path turns into something
like C:/dev/null, so the specs were still trying to manage their configuration
directories when this wasn't desired or handled by the spec.

Now, we also exclude management of C:/dev (or similar), to mimic the behavior
on Windows. Because this isn't a standard path (and thus will not be used for
anything else), there seems to be no harm in treating it as though it were
really /dev.

Reviewed-By: Josh Cooper &lt;josh@puppetlabs.com&gt;
(cherry picked from commit 44719fcf9f9053a7be1bea59d516f24d2234ede4)
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
FileSetting#to_resource had a provision to not attempt to manage files in /dev,
which effectively stopped spec from trying to make their configuration
directories in /dev/null. However, on Windows, the path turns into something
like C:/dev/null, so the specs were still trying to manage their configuration
directories when this wasn't desired or handled by the spec.

Now, we also exclude management of C:/dev (or similar), to mimic the behavior
on Windows. Because this isn't a standard path (and thus will not be used for
anything else), there seems to be no harm in treating it as though it were
really /dev.

Reviewed-By: Josh Cooper &lt;josh@puppetlabs.com&gt;
(cherry picked from commit 44719fcf9f9053a7be1bea59d516f24d2234ede4)
</pre>
</div>
</content>
</entry>
<entry>
<title>(#650) Allow symlinks for configuration directories</title>
<updated>2011-06-07T21:36:54+00:00</updated>
<author>
<name>Nick Lewis</name>
<email>nick@puppetlabs.com</email>
</author>
<published>2011-06-07T20:00:46+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=3f0dbb5fb7d9f4d88684fa07189270f42074e5f4'/>
<id>3f0dbb5fb7d9f4d88684fa07189270f42074e5f4</id>
<content type='text'>
Previously, pointing a setting like 'confdir' at a symlink to a directory would
replace the symlink with a directory. This was because files created by
settings implicitly managed links, rather than following them. This behavior is
now changed so that file settings will follow links.

The behavior of these symlinks is the same as any other file resource. That is,
if the target of the symlink doesn't exist, Puppet will consider this an
error. Similarly, if the target of the symlink is a file, then the symlink
will still be replaced with a directory, rather than replacing its target.

Reviewed-By: Jacob Helwig &lt;jacob@puppetlabs.com&gt;
Reviewed-By: Dominic Maraglia &lt;dominic@puppetlabs.com&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Previously, pointing a setting like 'confdir' at a symlink to a directory would
replace the symlink with a directory. This was because files created by
settings implicitly managed links, rather than following them. This behavior is
now changed so that file settings will follow links.

The behavior of these symlinks is the same as any other file resource. That is,
if the target of the symlink doesn't exist, Puppet will consider this an
error. Similarly, if the target of the symlink is a file, then the symlink
will still be replaced with a directory, rather than replacing its target.

Reviewed-By: Jacob Helwig &lt;jacob@puppetlabs.com&gt;
Reviewed-By: Dominic Maraglia &lt;dominic@puppetlabs.com&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Two space indentation</title>
<updated>2010-07-10T01:12:17+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:12:17+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=3180b9d9b2c844dade1d361326600f7001ec66dd'/>
<id>3180b9d9b2c844dade1d361326600f7001ec66dd</id>
<content type='text'>
Replaced 106806 occurances of ^( +)(.*$) with

The ruby community almost universally (i.e. everyone but Luke, Markus, and the other eleven people
who learned ruby in the 1900s) uses two-space indentation.

3 Examples:

    The code:
        end

        # Tell getopt which arguments are valid
        def test_get_getopt_args
            element = Setting.new :name =&gt; "foo", :desc =&gt; "anything", :settings =&gt; Puppet::Util::Settings.new
            assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")

    becomes:
        end

        # Tell getopt which arguments are valid
        def test_get_getopt_args
          element = Setting.new :name =&gt; "foo", :desc =&gt; "anything", :settings =&gt; Puppet::Util::Settings.new
          assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")

    The code:
            assert_equal(str, val)

            assert_instance_of(Float, result)

        end

        # Now test it with a passed object
    becomes:
          assert_equal(str, val)

          assert_instance_of(Float, result)

        end

        # Now test it with a passed object
    The code:
        end

        assert_nothing_raised do
            klass[:Yay] = "boo"
            klass["Cool"] = :yayness
        end

    becomes:
        end

        assert_nothing_raised do
          klass[:Yay] = "boo"
          klass["Cool"] = :yayness
        end
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Replaced 106806 occurances of ^( +)(.*$) with

The ruby community almost universally (i.e. everyone but Luke, Markus, and the other eleven people
who learned ruby in the 1900s) uses two-space indentation.

3 Examples:

    The code:
        end

        # Tell getopt which arguments are valid
        def test_get_getopt_args
            element = Setting.new :name =&gt; "foo", :desc =&gt; "anything", :settings =&gt; Puppet::Util::Settings.new
            assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")

    becomes:
        end

        # Tell getopt which arguments are valid
        def test_get_getopt_args
          element = Setting.new :name =&gt; "foo", :desc =&gt; "anything", :settings =&gt; Puppet::Util::Settings.new
          assert_equal([["--foo", GetoptLong::REQUIRED_ARGUMENT]], element.getopt_args, "Did not produce appropriate getopt args")

    The code:
            assert_equal(str, val)

            assert_instance_of(Float, result)

        end

        # Now test it with a passed object
    becomes:
          assert_equal(str, val)

          assert_instance_of(Float, result)

        end

        # Now test it with a passed object
    The code:
        end

        assert_nothing_raised do
            klass[:Yay] = "boo"
            klass["Cool"] = :yayness
        end

    becomes:
        end

        assert_nothing_raised do
          klass[:Yay] = "boo"
          klass["Cool"] = :yayness
        end
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Omit needless checks on defined</title>
<updated>2010-07-10T01:06:38+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:06:38+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=a07bbe2e711ee22a40e147c046997c8813ae3cc8'/>
<id>a07bbe2e711ee22a40e147c046997c8813ae3cc8</id>
<content type='text'>
* Replaced 53 occurances of

      defined\?\((.+?)\) (?:and|&amp;&amp;) \1( |$)

  with

      \1\2

  In code like:

     unless defined? @foo and @foo and bar("baz")

  "defined? @foo and @foo" can safely be replaced with "@foo":

     unless @foo and bar("baz")

  Because:

   * Both evaluate to false/nil when @foo is not defined
   * Both evaluate to @foo when @foo is defined

  3 Examples:

      The code:
          @sync = Sync.new unless defined?(@sync) and @sync
      becomes:
          @sync = Sync.new unless @sync
      The code:
          unless defined?(@content) and @content
      becomes:
          unless @content
      The code:
          raise(ArgumentError, "Already handling indirection for #{@indirection.name}; cannot also handle #{indirection}") if defined?(@indirection) and @indirection
      becomes:
          raise(ArgumentError, "Already handling indirection for #{@indirection.name}; cannot also handle #{indirection}") if @indirection

* Replaced 2 occurances of

      defined\?\((.+?)\) (?:and|&amp;&amp;) ! *\1.nil\?

  with

      !\1.nil?

  In code like:

     while defined? @foo and ! @foo.nil? ...

  "defined? @foo and ! @foo.nil?" can safely be replaced with "! @foo.nil?":

     while ! @foo.nil? ...

  Because:

   * Both evaluate to false/nil when @foo is not defined
   * Both evaluate to "! @foo.nil?" when @foo is defined

  2 Examples:

      The code:
          !!(defined?(@value) and ! @value.nil?)
      becomes:
          !!(!@value.nil?)
      The code:
          self.init unless defined?(@@state) and ! @@state.nil?
      becomes:
          self.init unless !@@state.nil?
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* Replaced 53 occurances of

      defined\?\((.+?)\) (?:and|&amp;&amp;) \1( |$)

  with

      \1\2

  In code like:

     unless defined? @foo and @foo and bar("baz")

  "defined? @foo and @foo" can safely be replaced with "@foo":

     unless @foo and bar("baz")

  Because:

   * Both evaluate to false/nil when @foo is not defined
   * Both evaluate to @foo when @foo is defined

  3 Examples:

      The code:
          @sync = Sync.new unless defined?(@sync) and @sync
      becomes:
          @sync = Sync.new unless @sync
      The code:
          unless defined?(@content) and @content
      becomes:
          unless @content
      The code:
          raise(ArgumentError, "Already handling indirection for #{@indirection.name}; cannot also handle #{indirection}") if defined?(@indirection) and @indirection
      becomes:
          raise(ArgumentError, "Already handling indirection for #{@indirection.name}; cannot also handle #{indirection}") if @indirection

* Replaced 2 occurances of

      defined\?\((.+?)\) (?:and|&amp;&amp;) ! *\1.nil\?

  with

      !\1.nil?

  In code like:

     while defined? @foo and ! @foo.nil? ...

  "defined? @foo and ! @foo.nil?" can safely be replaced with "! @foo.nil?":

     while ! @foo.nil? ...

  Because:

   * Both evaluate to false/nil when @foo is not defined
   * Both evaluate to "! @foo.nil?" when @foo is defined

  2 Examples:

      The code:
          !!(defined?(@value) and ! @value.nil?)
      becomes:
          !!(!@value.nil?)
      The code:
          self.init unless defined?(@@state) and ! @@state.nil?
      becomes:
          self.init unless !@@state.nil?
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Avoid explicit returns</title>
<updated>2010-07-10T01:06:33+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:06:33+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=8d1fbe4586c91682cdda0cb271649e918fd9778b'/>
<id>8d1fbe4586c91682cdda0cb271649e918fd9778b</id>
<content type='text'>
Replaced 583 occurances of

    (DEF)
        (LINES)
        return (.*)
    end

with

3 Examples:

    The code:
        def consolidate_failures(failed)
            filters = Hash.new { |h,k| h[k] = [] }
            failed.each do |spec, failed_trace|
                if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
                    filters[f] &lt;&lt; spec
                    break
                end
            end
            return filters
        end
    becomes:
        def consolidate_failures(failed)
            filters = Hash.new { |h,k| h[k] = [] }
            failed.each do |spec, failed_trace|
                if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
                    filters[f] &lt;&lt; spec
                    break
                end
            end
            filters
        end
    The code:
        def retrieve
            return_value = super
            return_value = return_value[0] if return_value &amp;&amp; return_value.is_a?(Array)

            return return_value
        end
    becomes:
        def retrieve
            return_value = super
            return_value = return_value[0] if return_value &amp;&amp; return_value.is_a?(Array)

            return_value
        end
    The code:
        def fake_fstab
            os = Facter['operatingsystem']
            if os == "Solaris"
                name = "solaris.fstab"
            elsif os == "FreeBSD"
                name = "freebsd.fstab"
            else
                # Catchall for other fstabs
                name = "linux.fstab"
            end
            oldpath = @provider_class.default_target
            return fakefile(File::join("data/types/mount", name))
        end
    becomes:
        def fake_fstab
            os = Facter['operatingsystem']
            if os == "Solaris"
                name = "solaris.fstab"
            elsif os == "FreeBSD"
                name = "freebsd.fstab"
            else
                # Catchall for other fstabs
                name = "linux.fstab"
            end
            oldpath = @provider_class.default_target
            fakefile(File::join("data/types/mount", name))
        end
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Replaced 583 occurances of

    (DEF)
        (LINES)
        return (.*)
    end

with

3 Examples:

    The code:
        def consolidate_failures(failed)
            filters = Hash.new { |h,k| h[k] = [] }
            failed.each do |spec, failed_trace|
                if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
                    filters[f] &lt;&lt; spec
                    break
                end
            end
            return filters
        end
    becomes:
        def consolidate_failures(failed)
            filters = Hash.new { |h,k| h[k] = [] }
            failed.each do |spec, failed_trace|
                if f = test_files_for(failed).find { |f| failed_trace =~ Regexp.new(f) }
                    filters[f] &lt;&lt; spec
                    break
                end
            end
            filters
        end
    The code:
        def retrieve
            return_value = super
            return_value = return_value[0] if return_value &amp;&amp; return_value.is_a?(Array)

            return return_value
        end
    becomes:
        def retrieve
            return_value = super
            return_value = return_value[0] if return_value &amp;&amp; return_value.is_a?(Array)

            return_value
        end
    The code:
        def fake_fstab
            os = Facter['operatingsystem']
            if os == "Solaris"
                name = "solaris.fstab"
            elsif os == "FreeBSD"
                name = "freebsd.fstab"
            else
                # Catchall for other fstabs
                name = "linux.fstab"
            end
            oldpath = @provider_class.default_target
            return fakefile(File::join("data/types/mount", name))
        end
    becomes:
        def fake_fstab
            os = Facter['operatingsystem']
            if os == "Solaris"
                name = "solaris.fstab"
            elsif os == "FreeBSD"
                name = "freebsd.fstab"
            else
                # Catchall for other fstabs
                name = "linux.fstab"
            end
            oldpath = @provider_class.default_target
            fakefile(File::join("data/types/mount", name))
        end
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Booleans are first class values.</title>
<updated>2010-07-10T01:06:12+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:06:12+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=889158ad57e33df083613d6f7d136b2e11aaa16a'/>
<id>889158ad57e33df083613d6f7d136b2e11aaa16a</id>
<content type='text'>
* Replaced 2 occurances of

      def (.*)
          begin
              (.*) = Integer\((.*)\)
              return \2
          rescue ArgumentError
              \2 = nil
          end
          if \2 = (.*)
              return \2
          else
              return false
          end
      end

  with

  2 Examples:

      The code:
          def validuser?(value)
              begin
                  number = Integer(value)
                  return number
              rescue ArgumentError
                  number = nil
              end
              if number = uid(value)
                  return number
              else
                  return false
              end
          end
      becomes:
          def validuser?(value)
              Integer(value) rescue uid(value) || false
          end
      The code:
          def validgroup?(value)
              begin
                  number = Integer(value)
                  return number
              rescue ArgumentError
                  number = nil
              end
              if number = gid(value)
                  return number
              else
                  return false
              end
          end
      becomes:
          def validgroup?(value)
              Integer(value) rescue gid(value) || false
          end

* Replaced 28 occurances of

      return (.*?) if (.*)
      return (.*)

  with

  3 Examples:

      The code:
          return send(options[:mode]) if [:rdoc, :trac, :markdown].include?(options[:mode])
          return other
      becomes:
          return[:rdoc, :trac, :markdown].include?(options[:mode]) ? send(options[:mode]) : other
      The code:
          return true if known_resource_types.definition(name)
          return false
      becomes:
          return(known_resource_types.definition(name) ? true : false)
      The code:
          return :rest if request.protocol == 'https'
          return Puppet::FileBucket::File.indirection.terminus_class
      becomes:
          return(request.protocol == 'https' ? :rest : Puppet::FileBucket::File.indirection.terminus_class)

* Replaced no occurances of

      return (.*?) unless (.*)
      return (.*)

  with

* Replaced 7 occurances of

      if (.*)
          (.*[^:])false
      else
          \2true
      end

  with

  3 Examples:

      The code:
          if RUBY_PLATFORM == "i386-mswin32"
              InstallOptions.ri  = false
          else
              InstallOptions.ri  = true
          end
      becomes:
          InstallOptions.ri  = RUBY_PLATFORM != "i386-mswin32"
      The code:
          if options[:references].length &gt; 1
              with_contents = false
          else
              with_contents = true
          end
      becomes:
          with_contents = options[:references].length &lt;= 1
      The code:
          if value == false or value == "" or value == :undef
              return false
          else
              return true
          end
      becomes:
          return (value != false and value != "" and value != :undef)

* Replaced 19 occurances of

      if (.*)
          (.*[^:])true
      else
          \2false
      end

  with

  3 Examples:

      The code:
          if Puppet::Util::Log.level == :debug
              return true
          else
              return false
          end
      becomes:
          return Puppet::Util::Log.level == :debug
      The code:
          if satisfies?(*features)
              return true
          else
              return false
          end
      becomes:
          return !!satisfies?(*features)
      The code:
          if self.class.parsed_auth_db.has_key?(resource[:name])
              return true
          else
              return false
          end
      becomes:
          return !!self.class.parsed_auth_db.has_key?(resource[:name])

* Replaced 1 occurance of

      if ([a-z_]) = (.*)
          (.*[^:])\1
      else
          \3(.*)
      end

  with

  1 Example:

      The code:
          if c = self.send(@subclassname, method)
              return c
          else
              return nil
          end
      becomes:
          return self.send(@subclassname, method) || nil

* Replaced 2 occurances of

      if (.*)
          (.*[^:])\1
      else
          \2false
      end

  with

  2 Examples:

      The code:
          if hash[:Local]
              @local = hash[:Local]
          else
              @local = false
          end
      becomes:
          @local = hash[:Local]
      The code:
          if hash[:Local]
              @local = hash[:Local]
          else
              @local = false
          end
      becomes:
          @local = hash[:Local]

* Replaced 10 occurances of

      if (.*)
          (.*[^:])(.*)
      else
          \2false
      end

  with

  3 Examples:

      The code:
          if defined?(@isnamevar)
              return @isnamevar
          else
              return false
          end
      becomes:
          return defined?(@isnamevar) &amp;&amp; @isnamevar
      The code:
          if defined?(@required)
              return @required
          else
              return false
          end
      becomes:
          return defined?(@required) &amp;&amp; @required
      The code:
          if number = uid(value)
              return number
          else
              return false
          end
      becomes:
          return (number = uid(value)) &amp;&amp; number

* Replaced no occurances of

      if (.*)
          (.*[^:])nil
      else
          \2(true)
      end

  with

* Replaced no occurances of

      if (.*)
          (.*[^:])true
      else
          \2nil
      end

  with

* Replaced no occurances of

      if (.*)
          (.*[^:])\1
      else
          \2nil
      end

  with

* Replaced 23 occurances of

      if (.*)
          (.*[^:])(.*)
      else
          \2nil
      end

  with

  3 Examples:

      The code:
          if node = Puppet::Node.find(hostname)
              env = node.environment
          else
              env = nil
          end
      becomes:
          env = (node = Puppet::Node.find(hostname)) ? node.environment : nil
      The code:
          if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?
              return @mounts[MODULES].copy(mod.name, mod.file_directory)
          else
              return nil
          end
      becomes:
          return (mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?) ? @mounts[MODULES].copy(mod.name, mod.file_directory) : nil
      The code:
          if hash.include?(:CA) and hash[:CA]
              @ca = Puppet::SSLCertificates::CA.new()
          else
              @ca = nil
          end
      becomes:
          @ca = (hash.include?(:CA) and hash[:CA]) ? Puppet::SSLCertificates::CA.new() : nil
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* Replaced 2 occurances of

      def (.*)
          begin
              (.*) = Integer\((.*)\)
              return \2
          rescue ArgumentError
              \2 = nil
          end
          if \2 = (.*)
              return \2
          else
              return false
          end
      end

  with

  2 Examples:

      The code:
          def validuser?(value)
              begin
                  number = Integer(value)
                  return number
              rescue ArgumentError
                  number = nil
              end
              if number = uid(value)
                  return number
              else
                  return false
              end
          end
      becomes:
          def validuser?(value)
              Integer(value) rescue uid(value) || false
          end
      The code:
          def validgroup?(value)
              begin
                  number = Integer(value)
                  return number
              rescue ArgumentError
                  number = nil
              end
              if number = gid(value)
                  return number
              else
                  return false
              end
          end
      becomes:
          def validgroup?(value)
              Integer(value) rescue gid(value) || false
          end

* Replaced 28 occurances of

      return (.*?) if (.*)
      return (.*)

  with

  3 Examples:

      The code:
          return send(options[:mode]) if [:rdoc, :trac, :markdown].include?(options[:mode])
          return other
      becomes:
          return[:rdoc, :trac, :markdown].include?(options[:mode]) ? send(options[:mode]) : other
      The code:
          return true if known_resource_types.definition(name)
          return false
      becomes:
          return(known_resource_types.definition(name) ? true : false)
      The code:
          return :rest if request.protocol == 'https'
          return Puppet::FileBucket::File.indirection.terminus_class
      becomes:
          return(request.protocol == 'https' ? :rest : Puppet::FileBucket::File.indirection.terminus_class)

* Replaced no occurances of

      return (.*?) unless (.*)
      return (.*)

  with

* Replaced 7 occurances of

      if (.*)
          (.*[^:])false
      else
          \2true
      end

  with

  3 Examples:

      The code:
          if RUBY_PLATFORM == "i386-mswin32"
              InstallOptions.ri  = false
          else
              InstallOptions.ri  = true
          end
      becomes:
          InstallOptions.ri  = RUBY_PLATFORM != "i386-mswin32"
      The code:
          if options[:references].length &gt; 1
              with_contents = false
          else
              with_contents = true
          end
      becomes:
          with_contents = options[:references].length &lt;= 1
      The code:
          if value == false or value == "" or value == :undef
              return false
          else
              return true
          end
      becomes:
          return (value != false and value != "" and value != :undef)

* Replaced 19 occurances of

      if (.*)
          (.*[^:])true
      else
          \2false
      end

  with

  3 Examples:

      The code:
          if Puppet::Util::Log.level == :debug
              return true
          else
              return false
          end
      becomes:
          return Puppet::Util::Log.level == :debug
      The code:
          if satisfies?(*features)
              return true
          else
              return false
          end
      becomes:
          return !!satisfies?(*features)
      The code:
          if self.class.parsed_auth_db.has_key?(resource[:name])
              return true
          else
              return false
          end
      becomes:
          return !!self.class.parsed_auth_db.has_key?(resource[:name])

* Replaced 1 occurance of

      if ([a-z_]) = (.*)
          (.*[^:])\1
      else
          \3(.*)
      end

  with

  1 Example:

      The code:
          if c = self.send(@subclassname, method)
              return c
          else
              return nil
          end
      becomes:
          return self.send(@subclassname, method) || nil

* Replaced 2 occurances of

      if (.*)
          (.*[^:])\1
      else
          \2false
      end

  with

  2 Examples:

      The code:
          if hash[:Local]
              @local = hash[:Local]
          else
              @local = false
          end
      becomes:
          @local = hash[:Local]
      The code:
          if hash[:Local]
              @local = hash[:Local]
          else
              @local = false
          end
      becomes:
          @local = hash[:Local]

* Replaced 10 occurances of

      if (.*)
          (.*[^:])(.*)
      else
          \2false
      end

  with

  3 Examples:

      The code:
          if defined?(@isnamevar)
              return @isnamevar
          else
              return false
          end
      becomes:
          return defined?(@isnamevar) &amp;&amp; @isnamevar
      The code:
          if defined?(@required)
              return @required
          else
              return false
          end
      becomes:
          return defined?(@required) &amp;&amp; @required
      The code:
          if number = uid(value)
              return number
          else
              return false
          end
      becomes:
          return (number = uid(value)) &amp;&amp; number

* Replaced no occurances of

      if (.*)
          (.*[^:])nil
      else
          \2(true)
      end

  with

* Replaced no occurances of

      if (.*)
          (.*[^:])true
      else
          \2nil
      end

  with

* Replaced no occurances of

      if (.*)
          (.*[^:])\1
      else
          \2nil
      end

  with

* Replaced 23 occurances of

      if (.*)
          (.*[^:])(.*)
      else
          \2nil
      end

  with

  3 Examples:

      The code:
          if node = Puppet::Node.find(hostname)
              env = node.environment
          else
              env = nil
          end
      becomes:
          env = (node = Puppet::Node.find(hostname)) ? node.environment : nil
      The code:
          if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?
              return @mounts[MODULES].copy(mod.name, mod.file_directory)
          else
              return nil
          end
      becomes:
          return (mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?) ? @mounts[MODULES].copy(mod.name, mod.file_directory) : nil
      The code:
          if hash.include?(:CA) and hash[:CA]
              @ca = Puppet::SSLCertificates::CA.new()
          else
              @ca = nil
          end
      becomes:
          @ca = (hash.include?(:CA) and hash[:CA]) ? Puppet::SSLCertificates::CA.new() : nil
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Line modifiers are preferred to one-line blocks.</title>
<updated>2010-07-10T01:06:06+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:06:06+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=81e283b28cdd91d259e3b60687aee7ea66e9d05d'/>
<id>81e283b28cdd91d259e3b60687aee7ea66e9d05d</id>
<content type='text'>
* 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]
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* 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]
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Use string interpolation</title>
<updated>2010-07-10T01:05:55+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:05:55+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=e8cf06336b64491a2dd7538a06651e0caaf6a48d'/>
<id>e8cf06336b64491a2dd7538a06651e0caaf6a48d</id>
<content type='text'>
* Replaced 83 occurances of

      (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[])

  with

      \1#{\2}"

  3 Examples:

      The code:
          puts "PUPPET " + status + ": " + process + ", " + state
      becomes:
          puts "PUPPET " + status + ": " + process + ", #{state}"
      The code:
          puts "PUPPET " + status + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      The code:
          }.compact.join( "\n" ) + "\n" + t + "]\n"
      becomes:
          }.compact.join( "\n" ) + "\n#{t}" + "]\n"

* Replaced 21 occurances of (.*)" *[+] *" with \1

  3 Examples:

      The code:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      The code:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      becomes:
          puts "PUPPET #{status}: #{process}, #{state}"
      The code:
          res = self.class.name + ": #{@name}" + "\n"
      becomes:
          res = self.class.name + ": #{@name}\n"

* Don't use string concatenation to split lines unless they would be very long.

  Replaced 11 occurances of

      (.*)(['"]) *[+]
       *(['"])(.*)

  with

  3 Examples:

      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified " +
              "Puppet process is running and the state file is no " +
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
      The code:
          o.separator   "Mandatory arguments to long options are mandatory for " +
          "short options too."
      becomes:
          o.separator   "Mandatory arguments to long options are mandatory for short options too."
      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
              "older than specified interval."
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval."

* Replaced no occurances of do (.*?) end with {\1}

* Replaced 1488 occurances of

      "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b)

  with

  20 Examples:

      The code:
          args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",")
      becomes:
          args[0].split(/\./).map do |s| "dc=#{s}" end.join(",")
      The code:
          puts "%s" % Puppet.version
      becomes:
          puts "#{Puppet.version}"
      The code:
          raise "Could not find information for %s" % node
      becomes:
          raise "Could not find information for #{node}"
      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
      The code:
          Puppet.err "Could not run %s: %s" % [client_class, detail]
      becomes:
          Puppet.err "Could not run #{client_class}: #{detail}"
      The code:
          raise "Could not find handler for %s" % arg
      becomes:
          raise "Could not find handler for #{arg}"
      The code:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}"
      The code:
          raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail
      becomes:
          raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
      The code:
          raise "Could not find facts for %s" % Puppet[:certname]
      becomes:
          raise "Could not find facts for #{Puppet[:certname]}"
      The code:
          raise ArgumentError, "%s is not readable" % path
      becomes:
          raise ArgumentError, "#{path} is not readable"
      The code:
          raise ArgumentError, "Invalid handler %s" % name
      becomes:
          raise ArgumentError, "Invalid handler #{name}"
      The code:
          debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str]
      becomes:
          debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
      The code:
          raise Puppet::Error, "unknown cert type '%s'" % hash[:type]
      becomes:
          raise Puppet::Error, "unknown cert type '#{hash[:type]}'"
      The code:
          Puppet.info "Creating a new certificate request for %s" % Puppet[:certname]
      becomes:
          Puppet.info "Creating a new certificate request for #{Puppet[:certname]}"
      The code:
          "Cannot create alias %s: object already exists" % [name]
      becomes:
          "Cannot create alias #{name}: object already exists"
      The code:
          return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum]
      becomes:
          return "replacing from source #{metadata.source} with contents #{metadata.checksum}"
      The code:
          it "should have a %s parameter" % param do
      becomes:
          it "should have a #{param} parameter" do
      The code:
          describe "when registring '%s' messages" % log do
      becomes:
          describe "when registring '#{log}' messages" do
      The code:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
      becomes:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
      The code:
          assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do
      becomes:
          assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* Replaced 83 occurances of

      (.*)" *[+] *([$@]?[\w_0-9.:]+?)(.to_s\b)?(?! *[*(%\w_0-9.:{\[])

  with

      \1#{\2}"

  3 Examples:

      The code:
          puts "PUPPET " + status + ": " + process + ", " + state
      becomes:
          puts "PUPPET " + status + ": " + process + ", #{state}"
      The code:
          puts "PUPPET " + status + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      The code:
          }.compact.join( "\n" ) + "\n" + t + "]\n"
      becomes:
          }.compact.join( "\n" ) + "\n#{t}" + "]\n"

* Replaced 21 occurances of (.*)" *[+] *" with \1

  3 Examples:

      The code:
          puts "PUPPET #{status}" + ": #{process}" + ", #{state}"
      becomes:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      The code:
          puts "PUPPET #{status}" + ": #{process}, #{state}"
      becomes:
          puts "PUPPET #{status}: #{process}, #{state}"
      The code:
          res = self.class.name + ": #{@name}" + "\n"
      becomes:
          res = self.class.name + ": #{@name}\n"

* Don't use string concatenation to split lines unless they would be very long.

  Replaced 11 occurances of

      (.*)(['"]) *[+]
       *(['"])(.*)

  with

  3 Examples:

      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified " +
              "Puppet process is running and the state file is no " +
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
      The code:
          o.separator   "Mandatory arguments to long options are mandatory for " +
          "short options too."
      becomes:
          o.separator   "Mandatory arguments to long options are mandatory for short options too."
      The code:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no " +
              "older than specified interval."
      becomes:
          o.define_head "The check_puppet Nagios plug-in checks that specified Puppet process is running and the state file is no older than specified interval."

* Replaced no occurances of do (.*?) end with {\1}

* Replaced 1488 occurances of

      "([^"\n]*%s[^"\n]*)" *% *(.+?)(?=$| *\b(do|if|while|until|unless|#)\b)

  with

  20 Examples:

      The code:
          args[0].split(/\./).map do |s| "dc=%s"%[s] end.join(",")
      becomes:
          args[0].split(/\./).map do |s| "dc=#{s}" end.join(",")
      The code:
          puts "%s" % Puppet.version
      becomes:
          puts "#{Puppet.version}"
      The code:
          raise "Could not find information for %s" % node
      becomes:
          raise "Could not find information for #{node}"
      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create #{dir}: basedir #{File.join(path)} is a file"
      The code:
          Puppet.err "Could not run %s: %s" % [client_class, detail]
      becomes:
          Puppet.err "Could not run #{client_class}: #{detail}"
      The code:
          raise "Could not find handler for %s" % arg
      becomes:
          raise "Could not find handler for #{arg}"
      The code:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file #{Puppet[:authconfig]}"
      The code:
          raise Puppet::Error, "Could not deserialize catalog from pson: %s" % detail
      becomes:
          raise Puppet::Error, "Could not deserialize catalog from pson: #{detail}"
      The code:
          raise "Could not find facts for %s" % Puppet[:certname]
      becomes:
          raise "Could not find facts for #{Puppet[:certname]}"
      The code:
          raise ArgumentError, "%s is not readable" % path
      becomes:
          raise ArgumentError, "#{path} is not readable"
      The code:
          raise ArgumentError, "Invalid handler %s" % name
      becomes:
          raise ArgumentError, "Invalid handler #{name}"
      The code:
          debug "Executing '%s' in zone %s with '%s'" % [command, @resource[:name], str]
      becomes:
          debug "Executing '#{command}' in zone #{@resource[:name]} with '#{str}'"
      The code:
          raise Puppet::Error, "unknown cert type '%s'" % hash[:type]
      becomes:
          raise Puppet::Error, "unknown cert type '#{hash[:type]}'"
      The code:
          Puppet.info "Creating a new certificate request for %s" % Puppet[:certname]
      becomes:
          Puppet.info "Creating a new certificate request for #{Puppet[:certname]}"
      The code:
          "Cannot create alias %s: object already exists" % [name]
      becomes:
          "Cannot create alias #{name}: object already exists"
      The code:
          return "replacing from source %s with contents %s" % [metadata.source, metadata.checksum]
      becomes:
          return "replacing from source #{metadata.source} with contents #{metadata.checksum}"
      The code:
          it "should have a %s parameter" % param do
      becomes:
          it "should have a #{param} parameter" do
      The code:
          describe "when registring '%s' messages" % log do
      becomes:
          describe "when registring '#{log}' messages" do
      The code:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration%stest" % l }
      becomes:
          paths = %w{a b c d e f g h}.collect { |l| "/tmp/iteration#{l}test" }
      The code:
          assert_raise(Puppet::Error, "Check '%s' did not fail on false" % check) do
      becomes:
          assert_raise(Puppet::Error, "Check '#{check}' did not fail on false") do
</pre>
</div>
</content>
</entry>
<entry>
<title>Code smell: Inconsistent indentation and related formatting issues</title>
<updated>2010-07-10T01:05:04+00:00</updated>
<author>
<name>Markus Roberts</name>
<email>Markus@reality.com</email>
</author>
<published>2010-07-10T01:05:04+00:00</published>
<link rel='alternate' type='text/html' href='https://fedorapeople.org/cgit/ricky/public_git/puppet.git/commit/?id=9ee56f2e67be973da49b1d3f21de1bf87de35e6f'/>
<id>9ee56f2e67be973da49b1d3f21de1bf87de35e6f</id>
<content type='text'>
* Replaced 163 occurances of

      defined\? +([@a-zA-Z_.0-9?=]+)

  with

      defined?(\1)

  This makes detecting subsequent patterns easier.

  3 Examples:

      The code:
          if ! defined? @parse_config
      becomes:
          if ! defined?(@parse_config)
      The code:
          return @option_parser if defined? @option_parser
      becomes:
          return @option_parser if defined?(@option_parser)
      The code:
          if defined? @local and @local
      becomes:
          if defined?(@local) and @local

* Eliminate trailing spaces.

  Replaced 428 occurances of ^(.*?) +$ with \1

  1 file was skipped.

      test/ral/providers/host/parsed.rb because 0

* Replace leading tabs with an appropriate number of spaces.

  Replaced 306 occurances of ^(\t+)(.*) with

  Tabs are not consistently expanded in all environments.

* Don't arbitrarily wrap on sprintf (%) operator.

  Replaced 143 occurances of

      (.*['"] *%)
       +(.*)

  with

  Splitting the line does nothing to aid clarity and hinders further refactorings.

  3 Examples:

      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" %
              [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      The code:
          Puppet.err "Will not start without authorization file %s" %
              Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      The code:
          $stderr.puts "Could not find host for PID %s with status %s" %
              [pid, $?.exitstatus]
      becomes:
          $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus]

* Don't break short arrays/parameter list in two.

  Replaced 228 occurances of

      (.*)
       +(.*)

  with

  3 Examples:

      The code:
          puts @format.wrap(type.provider(prov).doc,
                            :indent =&gt; 4, :scrub =&gt; true)
      becomes:
          puts @format.wrap(type.provider(prov).doc, :indent =&gt; 4, :scrub =&gt; true)
      The code:
          assert(FileTest.exists?(daily),
              "Did not make daily graph for %s" % type)
      becomes:
          assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type)
      The code:
          assert(prov.target_object(:first).read !~ /^notdisk/,
              "Did not remove thing from disk")
      becomes:
          assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk")

* If arguments must wrap, treat them all equally

  Replaced 510 occurances of

      lines ending in things like ...(foo, or ...(bar(1,3),

  with

      \1
          \2

  3 Examples:

      The code:
          midscope.to_hash(false),
      becomes:
          assert_equal(
      The code:
          botscope.to_hash(true),
      becomes:
          # bottomscope, then checking that we see the right stuff.
      The code:
          :path =&gt; link,
      becomes:

* Replaced 4516 occurances of ^( *)(.*) with

  The present code base is supposed to use four-space indentation.  In some places we failed
  to maintain that standard.  These should be fixed regardless of the 2 vs. 4 space question.

  15 Examples:

      The code:

          def run_comp(cmd)
             puts cmd
             results = []
             old_sync = $stdout.sync
             $stdout.sync = true
             line = []
             begin
                open("| #{cmd}", "r") do |f|
                  until f.eof? do
                    c = f.getc
      becomes:

          def run_comp(cmd)
              puts cmd
              results = []
              old_sync = $stdout.sync
              $stdout.sync = true
              line = []
              begin
                  open("| #{cmd}", "r") do |f|
                      until f.eof? do
                          c = f.getc
      The code:
                              s.gsub!(/.{4}/n, '\\\\u\&amp;')
                            }
              string.force_encoding(Encoding::UTF_8)
              string
            rescue Iconv::Failure =&gt; e
              raise GeneratorError, "Caught #{e.class}: #{e}"
            end
          else
            def utf8_to_pson(string) # :nodoc:
              string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&amp;] }
              string.gsub!(/(
      becomes:
                                      s.gsub!(/.{4}/n, '\\\\u\&amp;')
                                  }
                  string.force_encoding(Encoding::UTF_8)
                  string
              rescue Iconv::Failure =&gt; e
                  raise GeneratorError, "Caught #{e.class}: #{e}"
              end
          else
              def utf8_to_pson(string) # :nodoc:
                  string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&amp;] }
                  string.gsub!(/(
      The code:
              end
          }

          rvalues:      rvalue
                      | rvalues comma rvalue {
              if val[0].instance_of?(AST::ASTArray)
                  result = val[0].push(val[2])
              else
                  result = ast AST::ASTArray, :children =&gt; [val[0],val[2]]
              end
          }
      becomes:
              end
          }

          rvalues:      rvalue
              | rvalues comma rvalue {
                  if val[0].instance_of?(AST::ASTArray)
                  result = val[0].push(val[2])
              else
                  result = ast AST::ASTArray, :children =&gt; [val[0],val[2]]
              end
          }
      The code:
          #passwdproc = proc { @password }

                      keytext = @key.export(

              OpenSSL::Cipher::DES.new(:EDE3, :CBC),

              @password
          )
          File.open(@keyfile, "w", 0400) { |f|
              f &lt;&lt; keytext
          }
      becomes:
          #        passwdproc = proc { @password }

              keytext = @key.export(

                  OpenSSL::Cipher::DES.new(:EDE3, :CBC),

                  @password
                  )
                  File.open(@keyfile, "w", 0400) { |f|
                      f &lt;&lt; keytext
                  }
      The code:
          end

          def to_manifest
              "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                   @params.collect { |p, v|
                       if v.is_a? Array
                           "    #{p} =&gt; [\'#{v.join("','")}\']"
                       else
                           "    #{p} =&gt; \'#{v}\'"
                       end
                   }.join(",\n")
      becomes:
          end

          def to_manifest
              "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                  @params.collect { |p, v|
                      if v.is_a? Array
                          "    #{p} =&gt; [\'#{v.join("','")}\']"
                      else
                          "    #{p} =&gt; \'#{v}\'"
                      end
                  }.join(",\n")
      The code:
          via the augeas tool.

           Requires:
             - augeas to be installed (http://www.augeas.net)
             - ruby-augeas bindings

           Sample usage with a string::

              augeas{\"test1\" :
                     context =&gt; \"/files/etc/sysconfig/firstboot\",
                     changes =&gt; \"set RUN_FIRSTBOOT YES\",
      becomes:
          via the augeas tool.

          Requires:
              - augeas to be installed (http://www.augeas.net)
              - ruby-augeas bindings

          Sample usage with a string::

              augeas{\"test1\" :
                  context =&gt; \"/files/etc/sysconfig/firstboot\",
                  changes =&gt; \"set RUN_FIRSTBOOT YES\",
      The code:
              names.should_not be_include("root")
          end

          describe "when generating a purgeable resource" do
              it "should be included in the generated resources" do
                  Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                  @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
              end
          end

          describe "when the instance's do not have an ensure property" do
      becomes:
              names.should_not be_include("root")
          end

          describe "when generating a purgeable resource" do
              it "should be included in the generated resources" do
                  Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                  @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
              end
          end

          describe "when the instance's do not have an ensure property" do
      The code:
          describe "when the instance's do not have an ensure property" do
              it "should not be included in the generated resources" do
                  @no_ensure_resource = Puppet::Type.type(:exec).new(:name =&gt; '/usr/bin/env echo')
                  Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                  @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
              end
          end

          describe "when the instance's ensure property does not accept absent" do
              it "should not be included in the generated resources" do
                  @no_absent_resource = Puppet::Type.type(:service).new(:name =&gt; 'foobar')
      becomes:
          describe "when the instance's do not have an ensure property" do
              it "should not be included in the generated resources" do
                  @no_ensure_resource = Puppet::Type.type(:exec).new(:name =&gt; '/usr/bin/env echo')
                  Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                  @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
              end
          end

          describe "when the instance's ensure property does not accept absent" do
              it "should not be included in the generated resources" do
                  @no_absent_resource = Puppet::Type.type(:service).new(:name =&gt; 'foobar')
      The code:
          func = nil
          assert_nothing_raised do

                          func = Puppet::Parser::AST::Function.new(

                  :name =&gt; "template",
                  :ftype =&gt; :rvalue,

                  :arguments =&gt; AST::ASTArray.new(
                      :children =&gt; [stringobj(template)]
                  )
      becomes:
          func = nil
          assert_nothing_raised do

              func = Puppet::Parser::AST::Function.new(

                  :name =&gt; "template",
                  :ftype =&gt; :rvalue,

                  :arguments =&gt; AST::ASTArray.new(
                      :children =&gt; [stringobj(template)]
                  )
      The code:

                  assert(
              @store.allowed?("hostname.madstop.com", "192.168.1.50"),

          "hostname not allowed")

                  assert(
              ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),

          "subname name allowed")
      becomes:

          assert(
              @store.allowed?("hostname.madstop.com", "192.168.1.50"),

          "hostname not allowed")

              assert(
                  ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),

          "subname name allowed")
      The code:

          assert_nothing_raised {

                          server = Puppet::Network::Handler.fileserver.new(

                  :Local =&gt; true,

                  :Config =&gt; false
              )
          }

      becomes:

          assert_nothing_raised {

              server = Puppet::Network::Handler.fileserver.new(

                  :Local =&gt; true,

                  :Config =&gt; false
              )
          }

      The code:
               'yay',
                                               { :failonfail =&gt; false,
                                                 :uid =&gt; @user.uid,

                                                 :gid =&gt; @user.gid }
                                             ).returns('output')

          output = Puppet::Util::SUIDManager.run_and_capture 'yay',
                                                             @user.uid,
                                                             @user.gid
      becomes:
                  'yay',
                      { :failonfail =&gt; false,
                          :uid =&gt; @user.uid,

                          :gid =&gt; @user.gid }
                              ).returns('output')

          output = Puppet::Util::SUIDManager.run_and_capture 'yay',
              @user.uid,
              @user.gid
      The code:
                      ).times(1)
          pkg.provider.expects(
                          :aptget

                                  ).with(

                          '-y',
                          '-q',
                          'remove',

                          'faff'
      becomes:
                      ).times(1)
          pkg.provider.expects(
              :aptget

                  ).with(

                      '-y',
                      '-q',
                      'remove',

                      'faff'
      The code:
          johnny one two
          billy three four\n"

                  # Just parse and generate, to make sure it's isomorphic.
                  assert_nothing_raised do
                      assert_equal(text, @parser.to_file(@parser.parse(text)),
                          "parsing was not isomorphic")
                  end
              end

              def test_valid_attrs
      becomes:
          johnny one two
          billy three four\n"

          #         Just parse and generate, to make sure it's isomorphic.
          assert_nothing_raised do
              assert_equal(text, @parser.to_file(@parser.parse(text)),
                  "parsing was not isomorphic")
                  end
              end

              def test_valid_attrs
      The code:
                  "testing",
                              :onboolean =&gt; [true, "An on bool"],

                              :string =&gt; ["a string", "A string arg"]
                              )
          result = []
          should = []
          assert_nothing_raised("Add args failed") do
              @config.addargs(result)
          end
          @config.each do |name, element|
      becomes:
                  "testing",
                      :onboolean =&gt; [true, "An on bool"],

                      :string =&gt; ["a string", "A string arg"]
                      )
          result = []
          should = []
          assert_nothing_raised("Add args failed") do
              @config.addargs(result)
          end
          @config.each do |name, element|
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
* Replaced 163 occurances of

      defined\? +([@a-zA-Z_.0-9?=]+)

  with

      defined?(\1)

  This makes detecting subsequent patterns easier.

  3 Examples:

      The code:
          if ! defined? @parse_config
      becomes:
          if ! defined?(@parse_config)
      The code:
          return @option_parser if defined? @option_parser
      becomes:
          return @option_parser if defined?(@option_parser)
      The code:
          if defined? @local and @local
      becomes:
          if defined?(@local) and @local

* Eliminate trailing spaces.

  Replaced 428 occurances of ^(.*?) +$ with \1

  1 file was skipped.

      test/ral/providers/host/parsed.rb because 0

* Replace leading tabs with an appropriate number of spaces.

  Replaced 306 occurances of ^(\t+)(.*) with

  Tabs are not consistently expanded in all environments.

* Don't arbitrarily wrap on sprintf (%) operator.

  Replaced 143 occurances of

      (.*['"] *%)
       +(.*)

  with

  Splitting the line does nothing to aid clarity and hinders further refactorings.

  3 Examples:

      The code:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" %
              [dir, File.join(path)]
      becomes:
          raise Puppet::Error, "Cannot create %s: basedir %s is a file" % [dir, File.join(path)]
      The code:
          Puppet.err "Will not start without authorization file %s" %
              Puppet[:authconfig]
      becomes:
          Puppet.err "Will not start without authorization file %s" % Puppet[:authconfig]
      The code:
          $stderr.puts "Could not find host for PID %s with status %s" %
              [pid, $?.exitstatus]
      becomes:
          $stderr.puts "Could not find host for PID %s with status %s" % [pid, $?.exitstatus]

* Don't break short arrays/parameter list in two.

  Replaced 228 occurances of

      (.*)
       +(.*)

  with

  3 Examples:

      The code:
          puts @format.wrap(type.provider(prov).doc,
                            :indent =&gt; 4, :scrub =&gt; true)
      becomes:
          puts @format.wrap(type.provider(prov).doc, :indent =&gt; 4, :scrub =&gt; true)
      The code:
          assert(FileTest.exists?(daily),
              "Did not make daily graph for %s" % type)
      becomes:
          assert(FileTest.exists?(daily), "Did not make daily graph for %s" % type)
      The code:
          assert(prov.target_object(:first).read !~ /^notdisk/,
              "Did not remove thing from disk")
      becomes:
          assert(prov.target_object(:first).read !~ /^notdisk/, "Did not remove thing from disk")

* If arguments must wrap, treat them all equally

  Replaced 510 occurances of

      lines ending in things like ...(foo, or ...(bar(1,3),

  with

      \1
          \2

  3 Examples:

      The code:
          midscope.to_hash(false),
      becomes:
          assert_equal(
      The code:
          botscope.to_hash(true),
      becomes:
          # bottomscope, then checking that we see the right stuff.
      The code:
          :path =&gt; link,
      becomes:

* Replaced 4516 occurances of ^( *)(.*) with

  The present code base is supposed to use four-space indentation.  In some places we failed
  to maintain that standard.  These should be fixed regardless of the 2 vs. 4 space question.

  15 Examples:

      The code:

          def run_comp(cmd)
             puts cmd
             results = []
             old_sync = $stdout.sync
             $stdout.sync = true
             line = []
             begin
                open("| #{cmd}", "r") do |f|
                  until f.eof? do
                    c = f.getc
      becomes:

          def run_comp(cmd)
              puts cmd
              results = []
              old_sync = $stdout.sync
              $stdout.sync = true
              line = []
              begin
                  open("| #{cmd}", "r") do |f|
                      until f.eof? do
                          c = f.getc
      The code:
                              s.gsub!(/.{4}/n, '\\\\u\&amp;')
                            }
              string.force_encoding(Encoding::UTF_8)
              string
            rescue Iconv::Failure =&gt; e
              raise GeneratorError, "Caught #{e.class}: #{e}"
            end
          else
            def utf8_to_pson(string) # :nodoc:
              string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&amp;] }
              string.gsub!(/(
      becomes:
                                      s.gsub!(/.{4}/n, '\\\\u\&amp;')
                                  }
                  string.force_encoding(Encoding::UTF_8)
                  string
              rescue Iconv::Failure =&gt; e
                  raise GeneratorError, "Caught #{e.class}: #{e}"
              end
          else
              def utf8_to_pson(string) # :nodoc:
                  string = string.gsub(/["\\\x0-\x1f]/) { MAP[$&amp;] }
                  string.gsub!(/(
      The code:
              end
          }

          rvalues:      rvalue
                      | rvalues comma rvalue {
              if val[0].instance_of?(AST::ASTArray)
                  result = val[0].push(val[2])
              else
                  result = ast AST::ASTArray, :children =&gt; [val[0],val[2]]
              end
          }
      becomes:
              end
          }

          rvalues:      rvalue
              | rvalues comma rvalue {
                  if val[0].instance_of?(AST::ASTArray)
                  result = val[0].push(val[2])
              else
                  result = ast AST::ASTArray, :children =&gt; [val[0],val[2]]
              end
          }
      The code:
          #passwdproc = proc { @password }

                      keytext = @key.export(

              OpenSSL::Cipher::DES.new(:EDE3, :CBC),

              @password
          )
          File.open(@keyfile, "w", 0400) { |f|
              f &lt;&lt; keytext
          }
      becomes:
          #        passwdproc = proc { @password }

              keytext = @key.export(

                  OpenSSL::Cipher::DES.new(:EDE3, :CBC),

                  @password
                  )
                  File.open(@keyfile, "w", 0400) { |f|
                      f &lt;&lt; keytext
                  }
      The code:
          end

          def to_manifest
              "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                   @params.collect { |p, v|
                       if v.is_a? Array
                           "    #{p} =&gt; [\'#{v.join("','")}\']"
                       else
                           "    #{p} =&gt; \'#{v}\'"
                       end
                   }.join(",\n")
      becomes:
          end

          def to_manifest
              "%s { '%s':\n%s\n}" % [self.type.to_s, self.name,
                  @params.collect { |p, v|
                      if v.is_a? Array
                          "    #{p} =&gt; [\'#{v.join("','")}\']"
                      else
                          "    #{p} =&gt; \'#{v}\'"
                      end
                  }.join(",\n")
      The code:
          via the augeas tool.

           Requires:
             - augeas to be installed (http://www.augeas.net)
             - ruby-augeas bindings

           Sample usage with a string::

              augeas{\"test1\" :
                     context =&gt; \"/files/etc/sysconfig/firstboot\",
                     changes =&gt; \"set RUN_FIRSTBOOT YES\",
      becomes:
          via the augeas tool.

          Requires:
              - augeas to be installed (http://www.augeas.net)
              - ruby-augeas bindings

          Sample usage with a string::

              augeas{\"test1\" :
                  context =&gt; \"/files/etc/sysconfig/firstboot\",
                  changes =&gt; \"set RUN_FIRSTBOOT YES\",
      The code:
              names.should_not be_include("root")
          end

          describe "when generating a purgeable resource" do
              it "should be included in the generated resources" do
                  Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                  @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
              end
          end

          describe "when the instance's do not have an ensure property" do
      becomes:
              names.should_not be_include("root")
          end

          describe "when generating a purgeable resource" do
              it "should be included in the generated resources" do
                  Puppet::Type.type(:host).stubs(:instances).returns [@purgeable_resource]
                  @resources.generate.collect { |r| r.ref }.should include(@purgeable_resource.ref)
              end
          end

          describe "when the instance's do not have an ensure property" do
      The code:
          describe "when the instance's do not have an ensure property" do
              it "should not be included in the generated resources" do
                  @no_ensure_resource = Puppet::Type.type(:exec).new(:name =&gt; '/usr/bin/env echo')
                  Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                  @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
              end
          end

          describe "when the instance's ensure property does not accept absent" do
              it "should not be included in the generated resources" do
                  @no_absent_resource = Puppet::Type.type(:service).new(:name =&gt; 'foobar')
      becomes:
          describe "when the instance's do not have an ensure property" do
              it "should not be included in the generated resources" do
                  @no_ensure_resource = Puppet::Type.type(:exec).new(:name =&gt; '/usr/bin/env echo')
                  Puppet::Type.type(:host).stubs(:instances).returns [@no_ensure_resource]
                  @resources.generate.collect { |r| r.ref }.should_not include(@no_ensure_resource.ref)
              end
          end

          describe "when the instance's ensure property does not accept absent" do
              it "should not be included in the generated resources" do
                  @no_absent_resource = Puppet::Type.type(:service).new(:name =&gt; 'foobar')
      The code:
          func = nil
          assert_nothing_raised do

                          func = Puppet::Parser::AST::Function.new(

                  :name =&gt; "template",
                  :ftype =&gt; :rvalue,

                  :arguments =&gt; AST::ASTArray.new(
                      :children =&gt; [stringobj(template)]
                  )
      becomes:
          func = nil
          assert_nothing_raised do

              func = Puppet::Parser::AST::Function.new(

                  :name =&gt; "template",
                  :ftype =&gt; :rvalue,

                  :arguments =&gt; AST::ASTArray.new(
                      :children =&gt; [stringobj(template)]
                  )
      The code:

                  assert(
              @store.allowed?("hostname.madstop.com", "192.168.1.50"),

          "hostname not allowed")

                  assert(
              ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),

          "subname name allowed")
      becomes:

          assert(
              @store.allowed?("hostname.madstop.com", "192.168.1.50"),

          "hostname not allowed")

              assert(
                  ! @store.allowed?("name.sub.madstop.com", "192.168.0.50"),

          "subname name allowed")
      The code:

          assert_nothing_raised {

                          server = Puppet::Network::Handler.fileserver.new(

                  :Local =&gt; true,

                  :Config =&gt; false
              )
          }

      becomes:

          assert_nothing_raised {

              server = Puppet::Network::Handler.fileserver.new(

                  :Local =&gt; true,

                  :Config =&gt; false
              )
          }

      The code:
               'yay',
                                               { :failonfail =&gt; false,
                                                 :uid =&gt; @user.uid,

                                                 :gid =&gt; @user.gid }
                                             ).returns('output')

          output = Puppet::Util::SUIDManager.run_and_capture 'yay',
                                                             @user.uid,
                                                             @user.gid
      becomes:
                  'yay',
                      { :failonfail =&gt; false,
                          :uid =&gt; @user.uid,

                          :gid =&gt; @user.gid }
                              ).returns('output')

          output = Puppet::Util::SUIDManager.run_and_capture 'yay',
              @user.uid,
              @user.gid
      The code:
                      ).times(1)
          pkg.provider.expects(
                          :aptget

                                  ).with(

                          '-y',
                          '-q',
                          'remove',

                          'faff'
      becomes:
                      ).times(1)
          pkg.provider.expects(
              :aptget

                  ).with(

                      '-y',
                      '-q',
                      'remove',

                      'faff'
      The code:
          johnny one two
          billy three four\n"

                  # Just parse and generate, to make sure it's isomorphic.
                  assert_nothing_raised do
                      assert_equal(text, @parser.to_file(@parser.parse(text)),
                          "parsing was not isomorphic")
                  end
              end

              def test_valid_attrs
      becomes:
          johnny one two
          billy three four\n"

          #         Just parse and generate, to make sure it's isomorphic.
          assert_nothing_raised do
              assert_equal(text, @parser.to_file(@parser.parse(text)),
                  "parsing was not isomorphic")
                  end
              end

              def test_valid_attrs
      The code:
                  "testing",
                              :onboolean =&gt; [true, "An on bool"],

                              :string =&gt; ["a string", "A string arg"]
                              )
          result = []
          should = []
          assert_nothing_raised("Add args failed") do
              @config.addargs(result)
          end
          @config.each do |name, element|
      becomes:
                  "testing",
                      :onboolean =&gt; [true, "An on bool"],

                      :string =&gt; ["a string", "A string arg"]
                      )
          result = []
          should = []
          assert_nothing_raised("Add args failed") do
              @config.addargs(result)
          end
          @config.each do |name, element|
</pre>
</div>
</content>
</entry>
</feed>
