On Mon, Apr 25, 2011 at 9:20 PM, Pieter van de Bruggen
<[email protected]> wrote:
> Reviewed-By: Matt Robinson
>
> Signed-off-by: Pieter van de Bruggen <[email protected]>
May I also request the commit message contain a bit more information?
Reading this out of context as I process email, I have the following
questions. Is the commit message a reasonable place to expect this
sort of information, or is there a better place to look for it?
* How is this change meant to be used?
* When should I use this over the previous way of doing things?
* Why are we making this change? (Maybe just a quick copy / paste from
the ticket?)
* Do I need to refactor any of my faces actions based on this commit?
e.g. is it a significant API change?
> ---
> Local-branch: tickets/2.7.next/7220
> lib/puppet/interface/action.rb | 5 +++
> lib/puppet/interface/action_builder.rb | 10 +++++
> spec/unit/interface/action_builder_spec.rb | 55
> ++++++++++++++++++++++++++++
> spec/unit/interface/action_spec.rb | 55
> ++++++++++++++++++++++++++++
> 4 files changed, 125 insertions(+), 0 deletions(-)
>
> diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
> index 08bc0a3..f8eef69 100644
> --- a/lib/puppet/interface/action.rb
> +++ b/lib/puppet/interface/action.rb
> @@ -217,6 +217,11 @@ WRAPPER
> option
> end
>
> + def inherit_options_from(action)
> + options = action.options.map { |opt| action.get_option(opt, false) }
> + options.reject!(&:nil?).uniq.each { |option| add_option(option) }
> + end
> +
> def option?(name)
> @options.include? name.to_sym
> end
> diff --git a/lib/puppet/interface/action_builder.rb
> b/lib/puppet/interface/action_builder.rb
> index 2ffa387..fd8b085 100644
> --- a/lib/puppet/interface/action_builder.rb
> +++ b/lib/puppet/interface/action_builder.rb
> @@ -38,6 +38,16 @@ class Puppet::Interface::ActionBuilder
> @action.add_option(option)
> end
>
> + def inherit_options_from(action)
> + if action.is_a? Symbol
> + name = action
> + action = @face.get_action(name) or
> + raise ArgumentError, "This Face has no action named #{name}!"
> + end
> +
> + @action.inherit_options_from(action)
> + end
> +
> def default(value = true)
> @action.default = !!value
> end
> diff --git a/spec/unit/interface/action_builder_spec.rb
> b/spec/unit/interface/action_builder_spec.rb
> index bf7afa7..6f36d2b 100755
> --- a/spec/unit/interface/action_builder_spec.rb
> +++ b/spec/unit/interface/action_builder_spec.rb
> @@ -52,6 +52,61 @@ describe Puppet::Interface::ActionBuilder do
> end
> end
>
> + describe "#inherit_options_from" do
> + let :face do
> + Puppet::Interface.new(:face_with_some_options, '0.0.1') do
> + option '-w'
> +
> + action(:foo) do
> + option '-x', '--ex'
> + option '-y', '--why'
> + end
> +
> + action(:bar) do
> + option '-z', '--zee'
> + end
> +
> + action(:baz) do
> + option '-z', '--zed'
> + end
> + end
> + end
> +
> + it 'should add the options from the specified action' do
> + foo = face.get_action(:foo)
> + action = Puppet::Interface::ActionBuilder.build(face,
> :inherit_options) do
> + inherit_options_from foo
> + end
> + action.options.should == foo.options
> + end
> +
> + it 'should add the options from multiple actions' do
> + foo = face.get_action(:foo)
> + bar = face.get_action(:bar)
> + action = Puppet::Interface::ActionBuilder.build(face,
> :inherit_options) do
> + inherit_options_from foo
> + inherit_options_from bar
> + end
> + action.options.should == (foo.options + bar.options).uniq.sort
> + end
> +
> + it 'should permit symbolic names for actions in the same face' do
> + foo = face.get_action(:foo)
> + action = Puppet::Interface::ActionBuilder.build(face,
> :inherit_options) do
> + inherit_options_from :foo
> + end
> + action.options.should == foo.options
> + end
> +
> + it 'should raise a useful error if you supply a bad action name' do
> + expect do
> + Puppet::Interface::ActionBuilder.build(face, :inherit_options) do
> + inherit_options_from :nowhere
> + end
> + end.to raise_error /nowhere/
> + end
> + end
> +
> context "inline documentation" do
> it "should set the summary" do
> action = Puppet::Interface::ActionBuilder.build(face, :foo) do
> diff --git a/spec/unit/interface/action_spec.rb
> b/spec/unit/interface/action_spec.rb
> index 24826a6..735fbcb 100755
> --- a/spec/unit/interface/action_spec.rb
> +++ b/spec/unit/interface/action_spec.rb
> @@ -68,6 +68,61 @@ describe Puppet::Interface::Action do
> end
> end
>
> + describe "#inherit_options_from" do
> + let :face do
> + Puppet::Interface.new(:face_with_some_options, '0.0.1') do
> + option '-w'
> +
> + action(:foo) do
> + option '-x', '--ex'
> + option '-y', '--why'
> + end
> +
> + action(:bar) do
> + option '-z', '--zee'
> + end
> +
> + action(:baz) do
> + option '-z', '--zed'
> + end
> +
> + action(:noopts) do
> + # no options declared
> + end
> + end
> + end
> +
> + subject { action = face.action(:new_action) { } }
> +
> + it 'should add the options from the specified action' do
> + subject.inherit_options_from(foo = face.get_action(:foo))
> + subject.options.should == foo.options
> + end
> +
> + it 'should not die when the specified action has no options' do
> + original_options = subject.options
> + subject.inherit_options_from(face.get_action(:noopts))
> + subject.options.should == original_options
> + end
> +
> + it 'should add the options from multiple actions' do
> + subject.inherit_options_from(foo = face.get_action(:foo))
> + subject.inherit_options_from(bar = face.get_action(:bar))
> + subject.options.should == (foo.options + bar.options).uniq.sort
> + end
> +
> + it 'should not inherit face options' do
> + subject.expects(:add_option)
> + subject.expects(:add_option).with(face.get_option(:w)).never
> + subject.inherit_options_from(face.get_action(:bar))
> + end
> +
> + it 'should raise an error if inheritance would duplicate options' do
> + subject.inherit_options_from(face.get_action(:bar))
> + expect { subject.inherit_options_from(face.get_action(:baz)) }.to
> raise_error
> + end
> + end
> +
> describe "when invoking" do
> it "should be able to call other actions on the same object" do
> face = Puppet::Interface.new(:my_face, '0.0.1') do
> --
> 1.7.4.1
>
> --
> 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.
>
>
--
Jeff McCune
Professional Services, Puppet Labs
@0xEFF
--
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.
