Alternate user execute resource code review feedackadamedx/alternate-user-execute

3 files changed, 12 insertions, 12 deletions

diff --git a/lib/chef/mixin/user_identity.rb b/lib/chef/mixin/user_identity.rb
index c037adb441..8cb8f72ed6 100644
--- a/lib/chef/mixin/user_identity.rb
+++ b/lib/chef/mixin/user_identity.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Adam Edwards (<adamed@chef.io>)
-# Copyright:: Copyright (c) 2015 Chef Software, Inc.
+# Copyright:: Copyright (c) 2016 Chef Software, Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -71,10 +71,10 @@ class Chef
         { domain: domain, user: user }
       end
 
-      protected(:validate_identity)
-      protected(:validate_identity_platform)
-      protected(:validate_identity_syntax)
-      protected(:qualify_user)
+      private(:validate_identity)
+      private(:validate_identity_platform)
+      private(:validate_identity_syntax)
+      private(:qualify_user)
 
     end
   end
diff --git a/lib/chef/provider/execute.rb b/lib/chef/provider/execute.rb
index 1b032a939e..c2498ab90b 100644
--- a/lib/chef/provider/execute.rb
+++ b/lib/chef/provider/execute.rb
@@ -32,10 +32,6 @@ class Chef
 
       def_delegators :@new_resource, :command, :returns, :environment, :user, :domain, :password, :group, :cwd, :umask, :creates
 
-      def initialize(new_resource, run_context)
-        super
-      end
-
       def load_current_resource
         current_resource = Chef::Resource::Execute.new(new_resource.name)
         current_resource
diff --git a/lib/chef/provider/script.rb b/lib/chef/provider/script.rb
index 3f89672f21..5df31786bd 100644
--- a/lib/chef/provider/script.rb
+++ b/lib/chef/provider/script.rb
@@ -67,17 +67,21 @@ class Chef
       end
 
       def set_owner_and_group
-        # FileUtils itself implements a no-op if +user+ or +group+ are nil
-        # You can prove this by running FileUtils.chown(nil,nil,'/tmp/file')
-        # as an unprivileged user.
         if ! Chef::Platform.windows?
+          # FileUtils itself implements a no-op if +user+ or +group+ are nil
+          # You can prove this by running FileUtils.chown(nil,nil,'/tmp/file')
+          # as an unprivileged user.
           FileUtils.chown(new_resource.user, new_resource.group, script_file.path)
         else
+          # And on Windows also this is a no-op if there is no user specified.
           grant_alternate_user_read_access
         end
       end
 
       def grant_alternate_user_read_access
+        # Do nothing if an alternate user isn't specified -- the file
+        # will already have the correct permissions for the user as part
+        # of the default ACL behavior on Windows.
         return if new_resource.user.nil?
 
         # Duplicate the script file's existing DACL