+1 Sent via BlackBerry from T-Mobile -----Original Message----- From: Steven Jenkins <[email protected]>
Date: Tue, 08 Sep 2009 10:14:34 To: <[email protected]> Subject: [Puppet-dev] Re: [PATCH] Have instance methods return self to allow chaining. Luke Kanies wrote: > Can you go into a bit more detail on the various repercussions of this? > > I understand the idea, but we don't really do it anywhere else right > now, and I'm wondering if this is a theme we should start adopting, or > what. > > If it is, then it should probably be in many places, not just this > one, right? > The basic idea is to use a Functional programming style so that you can use the results of one computation as the input to another. Using a Unix shell example, it's a lot like using unix pipes: e.g., grep some-pattern /var/spool/some-log | sort | uniq -c | sort -nr | tail -1 instead of grep some-pattern /var/spool/some-log > a.out sort a.out > b.out uniq -c b.out > c.out sort -nr c.out > d.out tail -1 d.out Note that Rails also uses this style; e.g., it's common to see things like total = Product.find(product_list).sum(&:price) where the result of one method is the object that is used for the subsequent method. I haven't looked all through the Puppet source for this, but, yes, it would be good to be done everywhere, not just this one file. It would be a technically straightforward task to do (just tedious). I'd suggest either putting together a set of Redmine tasks, and perhaps to put it aside and apply for a Google Summer of Code student next year if no one has gotten to it by Feb/March. Steven Jenkins End Point Corporation > On Sep 3, 2009, at 10:57 AM, Steven Jenkins wrote: > >> --- >> lib/puppet/indirector/facts/facter.rb | 8 +------- >> lib/puppet/node/facts.rb | 5 ++++- >> spec/unit/indirector/facts/facter.rb | 4 ++-- >> spec/unit/node/facts.rb | 24 ++++++++++++++++-------- >> 4 files changed, 23 insertions(+), 18 deletions(-) >> >> diff --git a/lib/puppet/indirector/facts/facter.rb b/lib/puppet/ >> indirector/facts/facter.rb >> index 9df71fc..62ac3ed 100644 >> --- a/lib/puppet/indirector/facts/facter.rb >> +++ b/lib/puppet/indirector/facts/facter.rb >> @@ -64,13 +64,7 @@ class Puppet::Node::Facts::Facter < >> Puppet::Indirector::Code >> >> # Look a host's facts up in Facter. >> def find(request) >> - result = Puppet::Node::Facts.new(request.key, Facter.to_hash) >> - >> - result.add_local_facts >> - result.stringify >> - result.downcase_if_necessary >> - >> - result >> + Puppet::Node::Facts.new(request.key, >> Facter.to_hash).add_local_facts.stringify.downcase_if_necessary >> end >> >> def save(facts) >> diff --git a/lib/puppet/node/facts.rb b/lib/puppet/node/facts.rb >> index dca435c..861214a 100755 >> --- a/lib/puppet/node/facts.rb >> +++ b/lib/puppet/node/facts.rb >> @@ -24,6 +24,7 @@ class Puppet::Node::Facts >> def add_local_facts >> values["clientversion"] = Puppet.version.to_s >> values["environment"] ||= Puppet.settings[:environment] >> + self >> end >> >> def initialize(name, values = {}) >> @@ -34,12 +35,13 @@ class Puppet::Node::Facts >> end >> >> def downcase_if_necessary >> - return unless Puppet.settings[:downcasefacts] >> + return self unless Puppet.settings[:downcasefacts] >> >> Puppet.warning "DEPRECATION NOTICE: Fact downcasing is >> deprecated; please disable (20080122)" >> values.each do |fact, value| >> values[fact] = value.downcase if value.is_a?(String) >> end >> + self >> end >> >> # Convert all fact values into strings. >> @@ -47,6 +49,7 @@ class Puppet::Node::Facts >> values.each do |fact, value| >> values[fact] = value.to_s >> end >> + self >> end >> >> private >> diff --git a/spec/unit/indirector/facts/facter.rb b/spec/unit/ >> indirector/facts/facter.rb >> index ea68f63..9903912 100755 >> --- a/spec/unit/indirector/facts/facter.rb >> +++ b/spec/unit/indirector/facts/facter.rb >> @@ -57,7 +57,7 @@ describe Puppet::Node::Facts::Facter do >> it "should add local facts" do >> facts = Puppet::Node::Facts.new("foo") >> Puppet::Node::Facts.expects(:new).returns facts >> - facts.expects(:add_local_facts) >> + facts.expects(:add_local_facts).returns facts >> >> @facter.find(@request) >> end >> @@ -65,7 +65,7 @@ describe Puppet::Node::Facts::Facter do >> it "should convert all facts into strings" do >> facts = Puppet::Node::Facts.new("foo") >> Puppet::Node::Facts.expects(:new).returns facts >> - facts.expects(:stringify) >> + facts.expects(:stringify).returns facts >> >> @facter.find(@request) >> end >> diff --git a/spec/unit/node/facts.rb b/spec/unit/node/facts.rb >> index a6e227a..44fb931 100755 >> --- a/spec/unit/node/facts.rb >> +++ b/spec/unit/node/facts.rb >> @@ -9,26 +9,34 @@ describe Puppet::Node::Facts, "when indirecting" do >> @facts = Puppet::Node::Facts.new("me") >> end >> >> + it "add_local_facts should return an instance of >> Puppet::Node::Facts" do >> + @facts.add_local_facts.class.should == Puppet::Node::Facts >> + end >> + >> + it "stringify should return an instance of Puppet::Node::Facts" >> do >> + @facts.stringify.class.should == Puppet::Node::Facts >> + end >> + >> + it "downcase_if_necessary should return an instance of >> Puppet::Node::Facts" do >> + @facts.downcase_if_necessary.class.should == >> Puppet::Node::Facts >> + end >> + >> it "should be able to convert all fact values to strings" do >> @facts.values["one"] = 1 >> - @facts.stringify >> - @facts.values["one"].should == "1" >> + @facts.stringify.values["one"].should == "1" >> end >> >> it "should add the Puppet version as a 'clientversion' fact when >> adding local facts" do >> - @facts.add_local_facts >> - @facts.values["clientversion"].should == Puppet.version.to_s >> + @facts.add_local_facts.values["clientversion"].should == >> Puppet.version.to_s >> end >> >> it "should add the current environment as a fact if one is not >> set when adding local facts" do >> - @facts.add_local_facts >> - @facts.values["environment"].should == Puppet[:environment] >> + @facts.add_local_facts.values["environment"].should == >> Puppet[:environment] >> end >> >> it "should not replace any existing environment fact when adding >> local facts" do >> @facts.values["environment"] = "foo" >> - @facts.add_local_facts >> - @facts.values["environment"].should == "foo" >> + @facts.add_local_facts.values["environment"].should == "foo" >> end >> >> it "should be able to downcase fact values" do >> -- >> 1.6.3.3 >> >> > > --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
