From 213b2c9ff49811ede15e570c99d1660ba8168ee5 Mon Sep 17 00:00:00 2001 From: Noah Harrison Date: Fri, 19 Jul 2024 15:00:20 -0400 Subject: [PATCH] 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 | 51 ++----------------------------- gush.gemspec | 3 +- lib/gush/cli.rb | 2 +- lib/gush/cli/overview.rb | 2 ++ lib/gush/client.rb | 8 ++--- lib/gush/graph.rb | 4 +-- lib/gush/job.rb | 2 +- lib/gush/version.rb | 2 +- lib/gush/worker.rb | 2 +- lib/gush/workflow.rb | 5 +-- spec/features/integration_spec.rb | 2 +- spec/gush/workflow_spec.rb | 6 ++-- spec/spec_helper.rb | 10 +++--- 13 files changed, 27 insertions(+), 72 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 3c2a8ce..18b949b 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/gush.gemspec b/gush.gemspec index 14d1551..1cdc4f7 100644 --- a/gush.gemspec +++ b/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' diff --git a/lib/gush/cli.rb b/lib/gush/cli.rb index 681ed80..ef56436 100644 --- a/lib/gush/cli.rb +++ b/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 diff --git a/lib/gush/cli/overview.rb b/lib/gush/cli/overview.rb index ac82d9d..0372e6e 100644 --- a/lib/gush/cli/overview.rb +++ b/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 diff --git a/lib/gush/client.rb b/lib/gush/client.rb index c8e36d6..8636b2f 100644 --- a/lib/gush/client.rb +++ b/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 @@ -157,7 +157,7 @@ def enqueue_job(workflow_id, job) persist_job(workflow_id, job) queue = job.queue || configuration.namespace wait = job.wait - + if wait.present? Gush::Worker.set(queue: queue, wait: wait).perform_later(*[workflow_id, job.name]) else diff --git a/lib/gush/graph.rb b/lib/gush/graph.rb index d599816..46db8ad 100644 --- a/lib/gush/graph.rb +++ b/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 diff --git a/lib/gush/job.rb b/lib/gush/job.rb index e432648..8705fb2 100644 --- a/lib/gush/job.rb +++ b/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 diff --git a/lib/gush/version.rb b/lib/gush/version.rb index 3982b0b..ea85ec6 100644 --- a/lib/gush/version.rb +++ b/lib/gush/version.rb @@ -1,3 +1,3 @@ module Gush - VERSION = '3.0.0' + VERSION = '3.0.0'.freeze end diff --git a/lib/gush/worker.rb b/lib/gush/worker.rb index 6578cd1..98cbd32 100644 --- a/lib/gush/worker.rb +++ b/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) diff --git a/lib/gush/workflow.rb b/lib/gush/workflow.rb index e00c77f..2ce97ff 100644 --- a/lib/gush/workflow.rb +++ b/lib/gush/workflow.rb @@ -2,7 +2,8 @@ module Gush class Workflow - attr_accessor :id, :jobs, :stopped, :persisted, :arguments, :kwargs, :globals + attr_accessor :jobs, :stopped, :persisted, :arguments, :kwargs, :globals + attr_writer :id def initialize(*args, globals: nil, **kwargs) @id = id @@ -55,7 +56,7 @@ def persist! client.persist_workflow(self) end - def expire! (ttl=nil) + def expire!(ttl=nil) client.expire_workflow(self, ttl) end diff --git a/spec/features/integration_spec.rb b/spec/features/integration_spec.rb index e7544d9..558e7f0 100644 --- a/spec/features/integration_spec.rb +++ b/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 diff --git a/spec/gush/workflow_spec.rb b/spec/gush/workflow_spec.rb index 720323d..9d095c9 100644 --- a/spec/gush/workflow_spec.rb +++ b/spec/gush/workflow_spec.rb @@ -137,21 +137,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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 2e75290..a55bd4e 100644 --- a/spec/spec_helper.rb +++ b/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