Please review pull request #624: (#13737) Swap build_tree and format_tree method names opened by (kelseyhightower)
Description:
The Puppet::Module::Tool.build_tree andPuppet::Module::Tool.format_tree method names do not match their
behavior. Both methods actually do the inverse,Puppet::Module::Tool.build_tree formats a tree for output andPuppet::Module::Tool.format_tree builds the tree that is passed toPuppet::Module::Tool.build_tree. Even this commit message is
confusing.
This patch swaps the names and updates the related specs.
- Opened: Thu Apr 05 02:24:47 UTC 2012
- Based on: puppetlabs:2.7.x (d88f3e7c387eff270101604f2fec2e086f88ebdd)
- Requested merge: kelseyhightower:ticket/2.7.x/13637_rename_build_tree_to_format_tree (a2990a83f880cd422d1ebc96b6aa750e893fce3d)
Diff follows:
diff --git a/lib/puppet/face/module/install.rb b/lib/puppet/face/module/install.rb
index 39aab02..c3e0754 100644
--- a/lib/puppet/face/module/install.rb
+++ b/lib/puppet/face/module/install.rb
@@ -164,9 +164,9 @@
Puppet.err(return_value[:error][:multiline])
exit 1
else
- tree = Puppet::Module::Tool.format_tree(return_value[:installed_modules], return_value[:install_dir])
+ tree = Puppet::Module::Tool.build_tree(return_value[:installed_modules], return_value[:install_dir])
return_value[:install_dir] + "\n" +
- Puppet::Module::Tool.build_tree(tree)
+ Puppet::Module::Tool.format_tree(tree)
end
end
end
diff --git a/lib/puppet/face/module/list.rb b/lib/puppet/face/module/list.rb
index 2126474..e3acbd9 100644
--- a/lib/puppet/face/module/list.rb
+++ b/lib/puppet/face/module/list.rb
@@ -171,17 +171,17 @@
# since dependencies may be cyclical.
modules_by_num_requires = modules.sort_by {|m| m.required_by.size}
@seen = {}
- tree = list_format_tree(modules_by_num_requires, [], nil,
+ tree = list_build_tree(modules_by_num_requires, [], nil,
:label_unmet => true, :path => path, :label_invalid => false)
else
tree = []
modules.sort_by { |mod| mod.forge_name or mod.name }.each do |mod|
- tree << list_format_node(mod, path, :label_unmet => false,
+ tree << list_build_node(mod, path, :label_unmet => false,
:path => path, :label_invalid => true)
end
end
- output << Puppet::Module::Tool.build_tree(tree)
+ output << Puppet::Module::Tool.format_tree(tree)
end
output
@@ -224,10 +224,10 @@
# │ └── bodepd-create_resources (v0.0.1)
# └── puppetlabs-sqlite (v0.0.1)
#
- def list_format_tree(list, ancestors=[], parent=nil, params={})
+ def list_build_tree(list, ancestors=[], parent=nil, params={})
list.map do |mod|
next if @seen[(mod.forge_name or mod.name)]
- node = list_format_node(mod, parent, params)
+ node = list_build_node(mod, parent, params)
@seen[(mod.forge_name or mod.name)] = true
unless ancestors.include?(mod)
@@ -240,7 +240,7 @@ def list_format_tree(list, ancestors=[], parent=nil, params={})
str << "(#{colorize(:cyan, mis_mod[:version_constraint])})"
node[:dependencies] << { :text => str }
end
- node[:dependencies] += list_format_tree(mod.dependencies_as_modules,
+ node[:dependencies] += list_build_tree(mod.dependencies_as_modules,
ancestors + [mod], mod, params)
end
@@ -259,7 +259,7 @@ def list_format_tree(list, ancestors=[], parent=nil, params={})
#
# Returns a Hash
#
- def list_format_node(mod, parent, params)
+ def list_build_node(mod, parent, params)
str = ''
str << (mod.forge_name ? mod.forge_name.gsub('/', '-') : mod.name)
str << ' (' + colorize(:cyan, mod.version ? "v#{mod.version}" : '???') + ')'
diff --git a/lib/puppet/face/module/upgrade.rb b/lib/puppet/face/module/upgrade.rb
index eac79c1..e62a7c5 100644
--- a/lib/puppet/face/module/upgrade.rb
+++ b/lib/puppet/face/module/upgrade.rb
@@ -75,9 +75,9 @@
Puppet.err(return_value[:error][:multiline])
exit 0
else
- tree = Puppet::Module::Tool.format_tree(return_value[:affected_modules], return_value[:base_dir])
+ tree = Puppet::Module::Tool.build_tree(return_value[:affected_modules], return_value[:base_dir])
return_value[:base_dir] + "\n" +
- Puppet::Module::Tool.build_tree(tree)
+ Puppet::Module::Tool.format_tree(tree)
end
end
end
diff --git a/lib/puppet/module_tool.rb b/lib/puppet/module_tool.rb
index 6ac1eb9..a05ac8e 100644
--- a/lib/puppet/module_tool.rb
+++ b/lib/puppet/module_tool.rb
@@ -50,7 +50,7 @@ def self.find_module_root(path)
# Builds a formatted tree from a list of node hashes containing +:text+
# and +:dependencies+ keys.
- def self.build_tree(nodes, level = 0)
+ def self.format_tree(nodes, level = 0)
str = ''
nodes.each_with_index do |node, i|
last_node = nodes.length - 1 == i
@@ -62,7 +62,7 @@ def self.build_tree(nodes, level = 0)
str << (deps.empty? ? "─" : "┬")
str << " #{node[:text]}\n"
- branch = build_tree(deps, level + 1)
+ branch = format_tree(deps, level + 1)
branch.gsub!(/^#{indent} /, indent + '│') unless last_node
str << branch
end
@@ -70,7 +70,7 @@ def self.build_tree(nodes, level = 0)
return str
end
- def self.format_tree(mods, dir)
+ def self.build_tree(mods, dir)
mods.each do |mod|
version_string = mod[:version][:vstring].sub(/^(?!v)/, 'v')
@@ -81,7 +81,7 @@ def self.format_tree(mods, dir)
mod[:text] = "#{mod[:module]} (#{colorize(:cyan, version_string)})"
mod[:text] += " [#{mod[:path]}]" unless mod[:path] == dir
- format_tree(mod[:dependencies], dir)
+ build_tree(mod[:dependencies], dir)
end
end
end
diff --git a/spec/unit/module_tool_spec.rb b/spec/unit/module_tool_spec.rb
index 5ddb01f..1517f30 100644
--- a/spec/unit/module_tool_spec.rb
+++ b/spec/unit/module_tool_spec.rb
@@ -4,14 +4,14 @@
require 'puppet/module_tool'
describe Puppet::Module::Tool, :fails_on_windows => true do
- describe '.build_tree' do
+ describe '.format_tree' do
it 'should return an empty tree when given an empty list' do
- subject.build_tree([]).should == ''
+ subject.format_tree([]).should == ''
end
it 'should return a shallow when given a list without dependencies' do
list = [ { :text => 'first' }, { :text => 'second' }, { :text => 'third' } ]
- subject.build_tree(list).should == <<-TREE
+ subject.format_tree(list).should == <<-TREE
├── first
├── second
└── third
@@ -32,7 +32,7 @@
]
},
]
- subject.build_tree(list).should == <<-TREE
+ subject.format_tree(list).should == <<-TREE
└─┬ first
└─┬ second
└── third
@@ -54,7 +54,7 @@
},
{ :text => 'fourth' }
]
- subject.build_tree(list).should == <<-TREE
+ subject.format_tree(list).should == <<-TREE
├─┬ first
│ └─┬ second
│ └── third
@@ -77,7 +77,7 @@
]
}
]
- subject.build_tree(list).should == <<-TREE
+ subject.format_tree(list).should == <<-TREE
└─┬ first
├─┬ second
│ └── third
@@ -101,7 +101,7 @@
},
{ :text => 'fifth' }
]
- subject.build_tree(list).should == <<-TREE
+ subject.format_tree(list).should == <<-TREE
├─┬ first
│ ├─┬ second
│ │ └── third
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
