Addressing review comments - better naming, comments, removed some TODOs

6 files changed, 20 insertions, 19 deletions

diff --git a/lib/chef/knife/data_bag_create.rb b/lib/chef/knife/data_bag_create.rb
index 40921d7d09..f8a7619a8a 100644
--- a/lib/chef/knife/data_bag_create.rb
+++ b/lib/chef/knife/data_bag_create.rb
@@ -18,13 +18,13 @@
 #
 
 require 'chef/knife'
+require 'chef/knife/data_bag_secret_options'
 
 class Chef
   class Knife
     class DataBagCreate < Knife
-      include DataBagCommon
+      include DataBagSecretOptions
 
-      # TODO duplicating deps here and in the DataBagCommon module
       deps do
         require 'chef/data_bag'
         require 'chef/encrypted_data_bag_item'
diff --git a/lib/chef/knife/data_bag_edit.rb b/lib/chef/knife/data_bag_edit.rb
index 718aeedc29..55f09f55c9 100644
--- a/lib/chef/knife/data_bag_edit.rb
+++ b/lib/chef/knife/data_bag_edit.rb
@@ -18,11 +18,12 @@
 #
 
 require 'chef/knife'
+require 'chef/knife/data_bag_secret_options'
 
 class Chef
   class Knife
     class DataBagEdit < Knife
-      include DataBagCommon
+      include DataBagSecretOptions
 
       deps do
         require 'chef/data_bag_item'
diff --git a/lib/chef/knife/data_bag_from_file.rb b/lib/chef/knife/data_bag_from_file.rb
index daaede706d..598a935160 100644
--- a/lib/chef/knife/data_bag_from_file.rb
+++ b/lib/chef/knife/data_bag_from_file.rb
@@ -19,11 +19,13 @@
 
 require 'chef/knife'
 require 'chef/util/path_helper'
+require 'chef/knife/data_bag_secret_options'
 
 class Chef
   class Knife
     class DataBagFromFile < Knife
       include DataBagCommon
+      include DataBagSecretOptions
 
       deps do
         require 'chef/data_bag'
diff --git a/lib/chef/knife/data_bag_common.rb b/lib/chef/knife/data_bag_secret_options.rb
index 93f8c98624..8b9c947bac 100644
--- a/lib/chef/knife/data_bag_common.rb
+++ b/lib/chef/knife/data_bag_secret_options.rb
@@ -18,30 +18,27 @@
 
 require 'mixlib/cli'
 require 'chef/config'
-require 'chef/encrypted_data_bag_item'
 
 class Chef
   class Knife
-    module DataBagCommon
+    module DataBagSecretOptions
       include Mixlib::CLI
 
-      # TODO when https://github.com/opscode/mixlib-cli/pull/13 is fixed, we can make this a base class
-      # instead of a module
       def self.included(base)
         base.option :secret,
                :short => "-s SECRET",
                :long  => "--secret ",
-               :description => "The secret key to use to encrypt data bag item values",
+               :description => "The secret key to use to encrypt data bag item values.  Can also be defaulted in your knife.rb with the key 'secret'",
                :proc => Proc.new { |s| Chef::Config[:knife][:secret] = s }
 
         base.option :secret_file,
                :long => "--secret-file SECRET_FILE",
-               :description => "A file containing the secret key to use to encrypt data bag item values",
+               :description => "A file containing the secret key to use to encrypt data bag item values.  Can also be defaulted in your knife.rb with the key 'secret_file'",
                :proc => Proc.new { |sf| Chef::Config[:knife][:secret_file] = sf }
 
         base.option :encrypt,
                :long => "--encrypt",
-               :description => "Only use the secret configured in knife.rb when this is true",
+               :description => "If 'secret' or 'secret_file' is present in your knife.rb, then encrypt data bags using it",
                :boolean => true,
                :default => false
       end
@@ -66,6 +63,10 @@ class Chef
       end
 
       def read_secret
+        # Moving the non 'compile-time' requires into here to speed up knife command loading
+        # IE, if we are not running 'knife data bag *' we don't need to load 'chef/encrypted_data_bag_item'
+        require 'chef/encrypted_data_bag_item'
+
         if config[:secret]
           config[:secret]
         elsif config[:secret_file]
@@ -84,7 +85,6 @@ class Chef
           exit(1)
         end
 
-        # TODO is there validation on the knife.rb schema?  If so, this validation should go there
         if has_secret? && has_secret_file?
           ui.fatal("Please specify only one of 'secret' or 'secret_file' in your config")
           exit(1)
diff --git a/lib/chef/knife/data_bag_show.rb b/lib/chef/knife/data_bag_show.rb
index 2a759d1b43..05f831a62d 100644
--- a/lib/chef/knife/data_bag_show.rb
+++ b/lib/chef/knife/data_bag_show.rb
@@ -18,11 +18,12 @@
 #
 
 require 'chef/knife'
+require 'chef/knife/data_bag_secret_options'
 
 class Chef
   class Knife
     class DataBagShow < Knife
-      include DataBagCommon
+      include DataBagSecretOptions
 
       deps do
         require 'chef/data_bag'
diff --git a/spec/unit/knife/data_bag_common_spec.rb b/spec/unit/knife/data_bag_secret_options_spec.rb
index 3e4ebf5026..98e876a5db 100644
--- a/spec/unit/knife/data_bag_common_spec.rb
+++ b/spec/unit/knife/data_bag_secret_options_spec.rb
@@ -21,16 +21,13 @@ require 'chef/knife'
 require 'chef/config'
 require 'tempfile'
 
-class ExampleDataBag < Chef::Knife
-  include Chef::Knife::DataBagCommon
-
-  #banner "you must provide a banner"
-  #category "data bag"
+class ExampleDataBagCommand < Chef::Knife
+  include Chef::Knife::DataBagSecretOptions
 end
 
-describe Chef::Knife::DataBagCommon do
+describe Chef::Knife::DataBagSecretOptions do
   let(:example_db) do
-    k = ExampleDataBag.new
+    k = ExampleDataBagCommand.new
     allow(k.ui).to receive(:stdout).and_return(stdout)
     k
   end