Skip to content

Commit 492d2d8

Browse files
schisamoschisamo
committed
prefer accessor methods to accessing instance variables directly
This also fixes [FC017] in addition to providing better encapsulation.
1 parent 85e3184 commit 492d2d8

2 files changed

Lines changed: 39 additions & 39 deletions

File tree

‎providers/pip.rb‎

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,56 +32,56 @@ def whyrun_supported?
3232

3333
action :install do
3434
# If we specified a version, and it's not the current version, move to the specified version
35-
if @new_resource.version != nil && @new_resource.version != @current_resource.version
36-
install_version = @new_resource.version
35+
if new_resource.version != nil && new_resource.version != current_resource.version
36+
install_version = new_resource.version
3737
# If it's not installed at all, install it
38-
elsif @current_resource.version == nil
38+
elsif current_resource.version == nil
3939
install_version = candidate_version
4040
end
4141

4242
if install_version
43-
description = "install package #{@new_resource} version #{install_version}"
43+
description = "install package #{new_resource} version #{install_version}"
4444
converge_by(description) do
45-
Chef::Log.info("Installing #{@new_resource} version #{install_version}")
45+
Chef::Log.info("Installing #{new_resource} version #{install_version}")
4646
status = install_package(install_version)
4747
if status
48-
@new_resource.updated_by_last_action(true)
48+
new_resource.updated_by_last_action(true)
4949
end
5050
end
5151
end
5252
end
5353

5454
action :upgrade do
55-
if @current_resource.version != candidate_version
56-
orig_version = @current_resource.version || "uninstalled"
57-
description = "upgrade #{@current_resource} version from #{@current_resource.version} to #{candidate_version}"
55+
if current_resource.version != candidate_version
56+
orig_version = current_resource.version || "uninstalled"
57+
description = "upgrade #{current_resource} version from #{current_resource.version} to #{candidate_version}"
5858
converge_by(description) do
59-
Chef::Log.info("Upgrading #{@new_resource} version from #{orig_version} to #{candidate_version}")
59+
Chef::Log.info("Upgrading #{new_resource} version from #{orig_version} to #{candidate_version}")
6060
status = upgrade_package(candidate_version)
6161
if status
62-
@new_resource.updated_by_last_action(true)
62+
new_resource.updated_by_last_action(true)
6363
end
6464
end
6565
end
6666
end
6767

6868
action :remove do
6969
if removing_package?
70-
description = "remove package #{@new_resource}"
70+
description = "remove package #{new_resource}"
7171
converge_by(description) do
72-
Chef::Log.info("Removing #{@new_resource}")
73-
remove_package(@new_resource.version)
74-
@new_resource.updated_by_last_action(true)
72+
Chef::Log.info("Removing #{new_resource}")
73+
remove_package(new_resource.version)
74+
new_resource.updated_by_last_action(true)
7575
end
7676
end
7777
end
7878

7979
def removing_package?
80-
if @current_resource.version.nil?
80+
if current_resource.version.nil?
8181
false # nothing to remove
82-
elsif @new_resource.version.nil?
82+
elsif new_resource.version.nil?
8383
true # remove any version of a package
84-
elsif @new_resource.version == @current_resource.version
84+
elsif new_resource.version == current_resource.version
8585
true # remove the version we have
8686
else
8787
false # we don't have the version we want to remove
@@ -93,8 +93,8 @@ def removing_package?
9393
# so refactoring into core Chef should be easy
9494

9595
def load_current_resource
96-
@current_resource = Chef::Resource::PythonPip.new(@new_resource.name)
97-
@current_resource.package_name(@new_resource.package_name)
96+
@current_resource = Chef::Resource::PythonPip.new(new_resource.name)
97+
@current_resource.package_name(new_resource.package_name)
9898
@current_resource.version(nil)
9999

100100
unless current_installed_version.nil?
@@ -108,9 +108,9 @@ def current_installed_version
108108
@current_installed_version ||= begin
109109
delimeter = /==/
110110

111-
version_check_cmd = "#{which_pip(@new_resource)} freeze | grep -i '^#{@new_resource.package_name}=='"
111+
version_check_cmd = "#{which_pip(new_resource)} freeze | grep -i '^#{new_resource.package_name}=='"
112112
# incase you upgrade pip with pip!
113-
if @new_resource.package_name.eql?('pip')
113+
if new_resource.package_name.eql?('pip')
114114
delimeter = /\s/
115115
version_check_cmd = "pip --version"
116116
end
@@ -126,14 +126,14 @@ def candidate_version
126126
# `pip search` doesn't return versions yet
127127
# `pip list` may be coming soon:
128128
# https://bitbucket.org/ianb/pip/issue/197/option-to-show-what-version-would-be
129-
@new_resource.version||'latest'
129+
new_resource.version||'latest'
130130
end
131131
end
132132

133133
def install_package(version)
134134
# if a version isn't specified (latest), is a source archive (ex. http://my.package.repo/SomePackage-1.0.4.zip),
135135
# or from a VCS (ex. git+https://git.repo/some_pkg.git) then do not append a version as this will break the source link
136-
if version == 'latest' || @new_resource.name.downcase.start_with?('http:', 'https:') || ['git', 'hg', 'svn'].include?(@new_resource.name.downcase.split('+')[0])
136+
if version == 'latest' || new_resource.name.downcase.start_with?('http:', 'https:') || ['git', 'hg', 'svn'].include?(new_resource.name.downcase.split('+')[0])
137137
version = ''
138138
else
139139
version = "==#{version}"
@@ -142,19 +142,19 @@ def install_package(version)
142142
end
143143

144144
def upgrade_package(version)
145-
@new_resource.options "#{@new_resource.options} --upgrade"
145+
new_resource.options "#{new_resource.options} --upgrade"
146146
install_package(version)
147147
end
148148

149149
def remove_package(version)
150-
@new_resource.options "#{@new_resource.options} --yes"
150+
new_resource.options "#{new_resource.options} --yes"
151151
pip_cmd('uninstall')
152152
end
153153

154154
def pip_cmd(subcommand, version='')
155-
options = { :timeout => @new_resource.timeout, :user => @new_resource.user, :group => @new_resource.group }
156-
options[:environment] = { 'HOME' => ::File.expand_path("~#{@new_resource.user}") } if @new_resource.user
157-
shell_out!("#{which_pip(@new_resource)} #{subcommand} #{@new_resource.options} #{@new_resource.name}#{version}", options)
155+
options = { :timeout => new_resource.timeout, :user => new_resource.user, :group => new_resource.group }
156+
options[:environment] = { 'HOME' => ::File.expand_path("~#{new_resource.user}") } if new_resource.user
157+
shell_out!("#{which_pip(new_resource)} #{subcommand} #{new_resource.options} #{new_resource.name}#{version}", options)
158158
end
159159

160160
# TODO remove when provider is moved into Chef core

‎providers/virtualenv.rb‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ def whyrun_supported?
2828

2929
action :create do
3030
unless exists?
31-
Chef::Log.info("Creating virtualenv #{@new_resource} at #{@new_resource.path}")
32-
execute "#{virtualenv_cmd} --python=#{@new_resource.interpreter} #{@new_resource.options} #{@new_resource.path}" do
31+
Chef::Log.info("Creating virtualenv #{new_resource} at #{new_resource.path}")
32+
execute "#{virtualenv_cmd} --python=#{new_resource.interpreter} #{new_resource.options} #{new_resource.path}" do
3333
user new_resource.owner if new_resource.owner
3434
group new_resource.group if new_resource.group
3535
end
@@ -39,20 +39,20 @@ def whyrun_supported?
3939

4040
action :delete do
4141
if exists?
42-
description = "delete virtualenv #{@new_resource} at #{@new_resource.path}"
42+
description = "delete virtualenv #{new_resource} at #{new_resource.path}"
4343
converge_by(description) do
44-
Chef::Log.info("Deleting virtualenv #{@new_resource} at #{@new_resource.path}")
45-
FileUtils.rm_rf(@new_resource.path)
44+
Chef::Log.info("Deleting virtualenv #{new_resource} at #{new_resource.path}")
45+
FileUtils.rm_rf(new_resource.path)
4646
end
4747
end
4848
end
4949

5050
def load_current_resource
51-
@current_resource = Chef::Resource::PythonVirtualenv.new(@new_resource.name)
52-
@current_resource.path(@new_resource.path)
51+
@current_resource = Chef::Resource::PythonVirtualenv.new(new_resource.name)
52+
@current_resource.path(new_resource.path)
5353

5454
if exists?
55-
cstats = ::File.stat(@current_resource.path)
55+
cstats = ::File.stat(current_resource.path)
5656
@current_resource.owner(cstats.uid)
5757
@current_resource.group(cstats.gid)
5858
end
@@ -69,6 +69,6 @@ def virtualenv_cmd()
6969

7070
private
7171
def exists?
72-
::File.exist?(@current_resource.path) && ::File.directory?(@current_resource.path) \
73-
&& ::File.exists?("#{@current_resource.path}/bin/activate")
72+
::File.exist?(current_resource.path) && ::File.directory?(current_resource.path) \
73+
&& ::File.exists?("#{current_resource.path}/bin/activate")
7474
end

0 commit comments

Comments
 (0)