Enabled some rubocop cops and fixed related warnings

Chose cops that were very easy to update the code to adhere to and, in some cases,
relate to preventing bugs. Mostly avoided cops that involve more person code style
preferences (e.g. spacing, alignment, naming).

.rubocop.yml

@@ -14,15 +14,6 @@ Layout/ArgumentAlignment:
 Layout/CaseIndentation:
   Enabled: false
 
-Layout/EmptyLineAfterGuardClause:
-  Enabled: false
-
-Layout/EmptyLineAfterMagicComment:
-  Enabled: false
-
-Layout/EmptyLinesAroundAccessModifier:
-  Enabled: false
-
 Layout/EmptyLinesAroundBlockBody:
   Enabled: false
 
@@ -35,9 +26,6 @@ Layout/FirstHashElementIndentation:
 Layout/HashAlignment:
   Enabled: false
 
-Layout/SpaceAfterMethodName:
-  Enabled: false
-
 Layout/SpaceAroundEqualsInParameterDefault:
   Enabled: false
 
@@ -53,27 +41,13 @@ Layout/SpaceInsideBlockBraces:
 Layout/SpaceInsideHashLiteralBraces:
   Enabled: false
 
-Layout/TrailingWhitespace:
-  Enabled: false
-
-Lint/AmbiguousBlockAssociation:
-  Enabled: false
-
 Lint/ConstantDefinitionInBlock:
-  Enabled: false
-
-Lint/DuplicateMethods:
-  Enabled: false
-
-Lint/ParenthesesAsGroupedExpression:
-  Enabled: false
+  Exclude:
+    - spec/**/*
 
 Lint/RedundantSplatExpansion:
   Enabled: false
 
-Lint/ShadowingOuterLocalVariable:
-  Enabled: false
-
 Lint/ToJSON:
   Enabled: false
 
@@ -116,9 +90,6 @@ Style/BlockDelimiters:
 Style/ClassVars:
   Enabled: false
 
-Style/ColonMethodCall:
-  Enabled: false
-
 Style/CombinableLoops:
   Enabled: false
 
@@ -134,18 +105,9 @@ Style/EmptyCaseCondition:
 Style/EmptyMethod:
   Enabled: false
 
-Style/Encoding:
-  Enabled: false
-
-Style/ExpandPathArguments:
-  Enabled: false
-
 Style/FrozenStringLiteralComment:
   Enabled: false
 
-Style/GlobalStdStream:
-  Enabled: false
-
 Style/GuardClause:
   Enabled: false
 
@@ -161,9 +123,6 @@ Style/InverseMethods:
 Style/MethodCallWithoutArgsParentheses:
   Enabled: false
 
-Style/MutableConstant:
-  Enabled: false
-
 Style/NumericLiteralPrefix:
   Enabled: false
 
@@ -176,15 +135,9 @@ Style/RaiseArgs:
 Style/SafeNavigation:
   Enabled: false
 
-Style/SelfAssignment:
-  Enabled: false
-
 Style/SpecialGlobalVars:
   Enabled: false
 
-Style/StderrPuts:
-  Enabled: false
-
 Style/StringLiterals:
   Enabled: false
 

gush.gemspec

@@ -1,5 +1,4 @@
-# coding: utf-8
-lib = File.expand_path('../lib', __FILE__)
+lib = File.expand_path('lib', __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require_relative 'lib/gush/version'

lib/gush/cli.rb

@@ -101,7 +101,7 @@ def viz(class_or_id)
         begin
           workflow = class_or_id.constantize.new
         rescue NameError => e
-          STDERR.puts Paint["'#{class_or_id}' is not a valid workflow class or id", :red]
+          warn Paint["'#{class_or_id}' is not a valid workflow class or id", :red]
           exit 1
         end
       end

lib/gush/cli/overview.rb

@@ -34,6 +34,7 @@ def jobs_list(jobs)
       end
 
       private
+
       def rows
         [].tap do |rows|
           columns.each_pair do |name, value|
@@ -91,6 +92,7 @@ def job_to_list_element(job)
 
       def jobs_by_type(type)
         return sorted_jobs if type == :all
+
         jobs.select{|j| j.public_send("#{type}?") }
       end
 

lib/gush/client.rb

@@ -95,7 +95,7 @@ def find_workflow(id)
         keys = redis.scan_each(match: "gush.jobs.#{id}.*")
 
         nodes = keys.each_with_object([]) do |key, array|
-          array.concat redis.hvals(key).map { |json| Gush::JSON.decode(json, symbolize_keys: true) }
+          array.concat(redis.hvals(key).map { |json| Gush::JSON.decode(json, symbolize_keys: true) })
         end
 
         workflow_from_hash(hash, nodes)
@@ -142,13 +142,13 @@ def destroy_job(workflow_id, job)
     end
 
     def expire_workflow(workflow, ttl=nil)
-      ttl = ttl || configuration.ttl
+      ttl ||= configuration.ttl
       redis.expire("gush.workflows.#{workflow.id}", ttl)
       workflow.jobs.each {|job| expire_job(workflow.id, job, ttl) }
     end
 
     def expire_job(workflow_id, job, ttl=nil)
-      ttl = ttl || configuration.ttl
+      ttl ||= configuration.ttl
       redis.expire("gush.jobs.#{workflow_id}.#{job.klass}", ttl)
     end
 

lib/gush/graph.rb

@@ -4,7 +4,7 @@
 
 module Gush
   class Graph
-    attr_reader :workflow, :filename, :path, :start_node, :end_node
+    attr_reader :workflow, :filename, :start_node, :end_node
 
     def initialize(workflow, options = {})
       @workflow = workflow
@@ -32,7 +32,7 @@ def viz
       file_format = path.split('.')[-1]
       format = file_format if file_format.length == 3
 
-      Graphviz::output(@graph, path: path, format: format)
+      Graphviz.output(@graph, path: path, format: format)
     end
 
     def path

lib/gush/job.rb

@@ -3,7 +3,7 @@ class Job
     attr_accessor :workflow_id, :incoming, :outgoing, :params,
       :finished_at, :failed_at, :started_at, :enqueued_at, :payloads,
       :klass, :queue, :wait
-    attr_reader :id, :klass, :output_payload, :params
+    attr_reader :id, :output_payload
 
     def initialize(opts = {})
       options = opts.dup

lib/gush/version.rb

@@ -1,3 +1,3 @@
 module Gush
-  VERSION = '3.0.0'
+  VERSION = '3.0.0'.freeze
 end

lib/gush/worker.rb

@@ -30,7 +30,7 @@ def perform(workflow_id, job_id)
 
     private
 
-    attr_reader :client, :workflow_id, :job, :configuration
+    attr_reader :workflow_id, :job
 
     def client
       @client ||= Gush::Client.new(Gush.configuration)

lib/gush/workflow.rb

@@ -2,7 +2,8 @@
 
 module Gush
   class Workflow
-    attr_accessor :id, :jobs, :dependencies, :stopped, :persisted, :arguments, :kwargs, :globals
+    attr_accessor :jobs, :dependencies, :stopped, :persisted, :arguments, :kwargs, :globals
+    attr_writer :id
 
     def initialize(*args, globals: nil, internal_state: {}, **kwargs)
       @arguments = args
@@ -56,7 +57,7 @@ def persist!
       client.persist_workflow(self)
     end
 
-    def expire! (ttl=nil)
+    def expire!(ttl=nil)
       client.expire_workflow(self, ttl)
     end
 

spec/features/integration_spec.rb

@@ -136,7 +136,7 @@ def perform
 
     class SummaryJob < Gush::Job
       def perform
-        output payloads.map { |payload| payload[:output] }
+        output(payloads.map { |payload| payload[:output] })
       end
     end
 

spec/gush/workflow_spec.rb

@@ -41,7 +41,7 @@ def configure(*args, **kwargs)
         jobs: flow.jobs,
         dependencies: flow.dependencies,
         persisted: true,
-        stopped: true,
+        stopped: true
       }
 
       flow_copy = TestWorkflow.new(internal_state: internal_state)
@@ -61,23 +61,12 @@ def configure(*args)
         end
       end
 
-      expect(INTERNAL_SETUP_SPY).to receive(:some_method).never
+      expect(INTERNAL_SETUP_SPY).not_to receive(:some_method)
 
       flow = TestWorkflow.new(internal_state: { needs_setup: false })
     end
   end
 
-  describe "#status" do
-    context "when failed" do
-      it "returns :failed" do
-        flow = TestWorkflow.create
-        flow.find_job("Prepare").fail!
-        flow.persist!
-        expect(flow.reload.status).to eq(:failed)
-      end
-    end
-  end
-
   describe "#save" do
     context "workflow not persisted" do
       it "sets persisted to true" do
@@ -132,22 +121,35 @@ def configure(*args)
   end
 
   describe "#status" do
+    context "when failed" do
+      it "returns :failed" do
+        flow = TestWorkflow.create
+        flow.find_job("Prepare").fail!
+        flow.persist!
+        expect(flow.reload.status).to eq(:failed)
+      end
+    end
+
     it "returns failed" do
       subject.find_job('Prepare').fail!
       expect(subject.status).to eq(:failed)
     end
+
     it "returns running" do
       subject.find_job('Prepare').start!
       expect(subject.status).to eq(:running)
     end
+
     it "returns finished" do
       subject.jobs.each {|n| n.finish! }
       expect(subject.status).to eq(:finished)
     end
+
     it "returns stopped" do
       subject.stopped = true
       expect(subject.status).to eq(:stopped)
     end
+
     it "returns pending" do
       expect(subject.status).to eq(:pending)
     end
@@ -175,7 +177,7 @@ def configure(*args)
           "stopped" => false,
           "dependencies" => [{
             "from" => "FetchFirstJob",
-            "to" => job_with_id("PersistFirstJob"),
+            "to" => job_with_id("PersistFirstJob")
           }],
           "arguments" => ["arg1", "arg2"],
           "kwargs" => {"arg3" => 123},
@@ -196,21 +198,21 @@ def configure(*args)
       flow = Gush::Workflow.new
       flow.run(Gush::Job, params: { something: 1 })
       flow.save
-      expect(flow.jobs.first.params).to eq ({ something: 1 })
+      expect(flow.jobs.first.params).to eq({ something: 1 })
     end
 
     it "merges globals with params and passes them to the job, with job param taking precedence" do
       flow = Gush::Workflow.new(globals: { something: 2, global1: 123 })
       flow.run(Gush::Job, params: { something: 1 })
       flow.save
-      expect(flow.jobs.first.params).to eq ({ something: 1, global1: 123 })
+      expect(flow.jobs.first.params).to eq({ something: 1, global1: 123 })
     end
 
     it "allows passing wait param to the job" do
       flow = Gush::Workflow.new
       flow.run(Gush::Job, wait: 5.seconds)
       flow.save
-      expect(flow.jobs.first.wait).to eq (5.seconds)
+      expect(flow.jobs.first.wait).to eq(5.seconds)
     end
 
     context "when graph is empty" do

spec/spec_helper.rb

@@ -120,11 +120,11 @@ def job_with_id(job_name)
     clear_enqueued_jobs
     clear_performed_jobs
 
-    Gush.configure do |config|
-      config.redis_url        = REDIS_URL
-      config.gushfile         = GUSHFILE
-      config.locking_duration = defined?(locking_duration) ? locking_duration : 2
-      config.polling_interval = defined?(polling_interval) ? polling_interval : 0.3
+    Gush.configure do |c|
+      c.redis_url        = REDIS_URL
+      c.gushfile         = GUSHFILE
+      c.locking_duration = defined?(locking_duration) ? locking_duration : 2
+      c.polling_interval = defined?(polling_interval) ? polling_interval : 0.3
     end
   end