Some code-style fixes and documentation

4 files changed, 76 insertions, 17 deletions

diff --git a/lib/system_check.rb b/lib/system_check.rb
index 1d09b57911a..79603dfc7cb 100644
--- a/lib/system_check.rb
+++ b/lib/system_check.rb
@@ -1,12 +1,23 @@
+# Library to perform System Checks
+#
+# Every Check is implemented as its own class inherited from SystemCheck::BaseCheck
+# Execution coordination and boilerplate output is done by the SystemCheck::SimpleExecutor
+#
+# This structure decouples checks from Rake tasks and facilitates unit-testing
 module SystemCheck
-  def self.run(component, checks = {}, executor_klass = SimpleExecutor)
+  # Executes a bunch of checks for specified component
+  #
+  # @param [String] component name of the component relative to the checks being executed
+  # @param [Array<BaseCheck>] checks classes of corresponding checks to be executed in the same order
+  # @param [BaseExecutor] executor_klass optionally specifiy a different executor class
+  def self.run(component, checks = [], executor_klass = SimpleExecutor)
     unless executor_klass.is_a? BaseExecutor
       raise ArgumentError, 'Invalid executor'
     end
 
     executor = executor_klass.new(component)
-    executor.checks = checks.map do |check|
-      raise ArgumentError unless check.is_a? BaseCheck
+    checks.each do |check|
+      executor << check
     end
   end
 end
diff --git a/lib/system_check/base_check.rb b/lib/system_check/base_check.rb
index 6e9f7e509a9..d465711d719 100644
--- a/lib/system_check/base_check.rb
+++ b/lib/system_check/base_check.rb
@@ -1,41 +1,71 @@
 module SystemCheck
+  # Base class for Checks. You must inherit from here
+  # and implement the methods below when necessary
   class BaseCheck
+    # This is where you should implement the main logic that will return
+    # a boolean at the end
+    #
+    # You should not print any output to STDOUT here, use the specific methods instead
+    #
+    # @return [Boolean] whether the check passed or not
     def check?
       raise NotImplementedError
     end
 
+    # This is where you should print detailed information for any error found during #check?
+    #
+    # You may use helper methods to help format the output:
+    #
+    # @see #try_fixing_it
+    # @see #fix_and_rerun
+    # @see #for_more_infromation
     def show_error
       raise NotImplementedError
     end
 
+    # If skip returns true, than no other method on this check will be executed
+    #
+    # @return [Boolean] whether or not this check should be skipped
     def skip?
       false
     end
 
+    # If you enabled #skip? here is where you define a custom message explaining why
+    #
+    # Do not print anything to STDOUT, return a string.
+    #
+    # @return [String] message why this check was skipped
     def skip_message
     end
 
     protected
 
+    # Display a formatted list of instructions on how to fix the issue identified by the #check?
+    #
+    # @param [Array<String>] steps one or short sentences with help how to fix the issue
     def try_fixing_it(*steps)
       steps = steps.shift if steps.first.is_a?(Array)
 
-      puts '  Try fixing it:'.color(:blue)
+      $stdout.puts '  Try fixing it:'.color(:blue)
       steps.each do |step|
-        puts "  #{step}"
+        $stdout.puts "  #{step}"
       end
     end
 
+    # Display a message telling to fix and rerun the checks
     def fix_and_rerun
-      puts '  Please fix the error above and rerun the checks.'.color(:red)
+      $stdout.puts '  Please fix the error above and rerun the checks.'.color(:red)
     end
 
+    # Display a formatted list of references (documentation or links) where to find more information
+    #
+    # @param [Array<String>] sources one or more references (documentation or links)
     def for_more_information(*sources)
       sources = sources.shift if sources.first.is_a?(Array)
 
-      puts '  For more information see:'.color(:blue)
+      $stdout.puts '  For more information see:'.color(:blue)
       sources.each do |source|
-        puts '  #{source}'
+        $stdout.puts '  #{source}'
       end
     end
   end
diff --git a/lib/system_check/base_executor.rb b/lib/system_check/base_executor.rb
index 18319843dea..2c2b33461d9 100644
--- a/lib/system_check/base_executor.rb
+++ b/lib/system_check/base_executor.rb
@@ -1,8 +1,11 @@
 module SystemCheck
+  # @attr_reader [Array<BaseCheck>] checks classes of corresponding checks to be executed in the same order
+  # @attr_reader [String] component name of the component relative to the checks being executed
   class BaseExecutor
     attr_reader :checks
     attr_reader :component
 
+    # @param [String] component name of the component relative to the checks being executed
     def initialize(component)
       raise ArgumentError unless component.is_a? String
 
@@ -10,6 +13,9 @@ module SystemCheck
       @checks = Set.new
     end
 
+    # Add a check to be executed
+    #
+    # @param [BaseCheck] check class
     def <<(check)
       raise ArgumentError unless check.is_a? BaseCheck
       @checks << check
diff --git a/lib/system_check/simple_executor.rb b/lib/system_check/simple_executor.rb
index 2ffe837e326..fc07f09dcb2 100644
--- a/lib/system_check/simple_executor.rb
+++ b/lib/system_check/simple_executor.rb
@@ -1,16 +1,22 @@
 module SystemCheck
+  # Simple Executor is current default executor for GitLab
+  # It is a simple port from display logic in the old check.rake
+  #
+  # There is no concurrency level and the output is progressively
+  # printed into the STDOUT
   class SimpleExecutor < BaseExecutor
+    # Executes defined checks in the specified order and outputs confirmation or error information
     def execute
       start_checking(component)
 
       @checks.each do |check|
-        print "#{check.name}"
+        $stdout.print "#{check.name}"
         if check.skip?
-          puts "skipped #{'('+skip_message+')' if skip_message}".color(:magenta)
+          $stdout.puts "skipped #{'(' + skip_message + ')' if skip_message}".color(:magenta)
         elsif check.check?
-          puts 'yes'.color(:green)
+          $stdout.puts 'yes'.color(:green)
         else
-          puts 'no'.color(:red)
+          $stdout.puts 'no'.color(:red)
           check.show_error
         end
       end
@@ -20,15 +26,21 @@ module SystemCheck
 
     private
 
+    # Prints header content for the series of checks to be executed for this component
+    #
+    # @param [String] component name of the component relative to the checks being executed
     def start_checking(component)
-      puts "Checking #{component.color(:yellow)} ..."
-      puts ''
+      $stdout.puts "Checking #{component.color(:yellow)} ..."
+      $stdout.puts ''
     end
 
+    # Prints footer content for the series of checks executed for this component
+    #
+    # @param [String] component name of the component relative to the checks being executed
     def finished_checking(component)
-      puts ''
-      puts "Checking #{component.color(:yellow)} ... #{"Finished".color(:green)}"
-      puts ''
+      $stdout.puts ''
+      $stdout.puts "Checking #{component.color(:yellow)} ... #{'Finished'.color(:green)}"
+      $stdout.puts ''
     end
   end
 end