Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add initial Sorbet linting; greatly improve YARD docs #235

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ require:
- rubocop-performance
- rubocop-rails
- rubocop-rspec
- rubocop-sorbet

inherit_mode:
merge:
Expand Down Expand Up @@ -85,6 +86,12 @@ RSpec/NestedGroups:
RSpec/MultipleMemoizedHelpers:
Enabled: false

Sorbet/FalseSigil:
Exclude:
- .mdstyle.rb
- .yard_support.rb
- spec/**/*

Style/Documentation:
Enabled: false

Expand Down
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ platforms :ruby do
gem "memory_profiler"
gem "pry-byebug"
gem "rbtrace"
gem "sorbet"
gem "sorbet-rails"
gem "sorbet-runtime"
gem "sord", github: "AaronC81/sord" # https://github.com/AaronC81/sord/pull/127b
gem "tapioca"

group :lint do
gem "erb_lint", ">= 0.0.35"
Expand All @@ -28,5 +33,6 @@ platforms :ruby do
gem "rubocop-performance"
gem "rubocop-rails"
gem "rubocop-rspec"
gem "rubocop-sorbet"
end
end
65 changes: 65 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
GIT
remote: https://github.com/AaronC81/sord.git
revision: 97c28d6589257e4fc3939241243db4c236a6f1b9
specs:
sord (3.0.1)
commander (~> 4.5)
parlour (~> 5.0)
sorbet-runtime
yard

GIT
remote: https://github.com/excid3/appraisal.git
revision: 8bb003b273ae074356dc67b59ecc67c8ae2f3a27
Expand All @@ -16,6 +26,7 @@ PATH
activerecord (>= 5.2.0)
concurrent-ruby (>= 1.0.2)
railties (>= 5.2.0)
sorbet-runtime-stub
thor (>= 0.14.1)
zeitwerk (>= 2.0)

Expand Down Expand Up @@ -123,6 +134,9 @@ GEM
chef-utils (16.11.7)
childprocess (3.0.0)
coderay (1.1.3)
colorize (0.8.1)
commander (4.6.0)
highline (~> 2.0.0)
concurrent-ruby (1.1.8)
console (1.10.2)
fiber-local
Expand Down Expand Up @@ -168,6 +182,7 @@ GEM
retriable (~> 3.0)
globalid (0.4.2)
activesupport (>= 4.2.0)
highline (2.0.3)
html_tokenizer (0.0.7)
i18n (1.8.9)
concurrent-ruby (~> 1.0)
Expand Down Expand Up @@ -212,9 +227,15 @@ GEM
sawyer (~> 0.8.0, >= 0.5.3)
optimist (3.0.1)
parallel (1.20.1)
parlour (5.0.0)
commander (~> 4.5)
parser
rainbow (~> 3.0)
sorbet-runtime (>= 0.5)
parser (3.0.0.0)
ast (~> 2.4.1)
pg (1.2.3)
polyfill (1.9.0)
protocol-hpack (1.4.2)
protocol-http (0.21.0)
protocol-http1 (0.13.2)
Expand Down Expand Up @@ -317,9 +338,12 @@ GEM
rubocop-rspec (2.2.0)
rubocop (~> 1.0)
rubocop-ast (>= 1.1.0)
rubocop-sorbet (0.6.1)
rubocop
ruby-progressbar (1.11.0)
ruby2_keywords (0.0.4)
rubyzip (2.3.0)
safe_type (1.1.1)
sawyer (0.8.2)
addressable (>= 2.3.5)
faraday (> 0.8, < 2.0)
Expand All @@ -328,6 +352,33 @@ GEM
rubyzip (>= 1.2.2)
sigdump (0.2.4)
smart_properties (1.15.0)
sorbet (0.5.6371)
sorbet-static (= 0.5.6371)
sorbet-coerce (0.4.0)
polyfill (~> 1.8)
safe_type (~> 1.1, >= 1.1.1)
sorbet-runtime (>= 0.4.4704)
sorbet-rails (0.7.3)
method_source (>= 0.9.2)
parlour (>= 4.0.1)
parser (>= 2.7)
sorbet-coerce (>= 0.2.6)
sorbet-runtime (>= 0.5)
sorbet-runtime (0.5.6371)
sorbet-runtime-stub (0.2.0)
sorbet-static (0.5.6371-universal-darwin-14)
sorbet-static (0.5.6371-universal-darwin-15)
sorbet-static (0.5.6371-universal-darwin-16)
sorbet-static (0.5.6371-universal-darwin-17)
sorbet-static (0.5.6371-universal-darwin-18)
sorbet-static (0.5.6371-universal-darwin-19)
sorbet-static (0.5.6371-universal-darwin-20)
sorbet-static (0.5.6371-x86_64-linux)
spoom (1.1.0)
colorize
sorbet (>= 0.5.6347)
sorbet-runtime
thor (>= 0.19.2)
spoon (0.0.6)
ffi
sprockets (4.0.2)
Expand All @@ -337,6 +388,14 @@ GEM
actionpack (>= 4.0)
activesupport (>= 4.0)
sprockets (>= 3.0.0)
tapioca (0.4.20)
bundler (>= 1.17.3)
parlour (>= 2.1.0)
pry (>= 0.12.2)
sorbet-runtime
sorbet-static (>= 0.4.4471)
spoom
thor (>= 0.19.2)
thor (1.1.0)
timers (4.3.3)
tomlrb (2.0.1)
Expand Down Expand Up @@ -386,8 +445,14 @@ DEPENDENCIES
rubocop-performance
rubocop-rails
rubocop-rspec
rubocop-sorbet
selenium-webdriver
sigdump
sorbet
sorbet-rails
sorbet-runtime
sord!
tapioca
yard
yard-activesupport-concern

Expand Down
3 changes: 3 additions & 0 deletions bin/lint
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ FileUtils.chdir GEM_ROOT do

system! "bundle exec rubocop #{flags.join(' ')}"

puts "\n== Sorbet Type Checking =="
system! "bundle exec srb tc"

puts "\n== ERB Lint =="
if options[:nofix]
system!("bundle exec erblint engine/app/views")
Expand Down
1 change: 1 addition & 0 deletions engine/app/controllers/good_job/active_jobs_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: true
module GoodJob
class ActiveJobsController < GoodJob::BaseController
def show
Expand Down
1 change: 1 addition & 0 deletions engine/app/controllers/good_job/base_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: strict
module GoodJob
class BaseController < ActionController::Base # rubocop:disable Rails/ApplicationController
protect_from_forgery with: :exception
Expand Down
1 change: 1 addition & 0 deletions engine/app/controllers/good_job/dashboards_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: true
module GoodJob
class DashboardsController < GoodJob::BaseController
class JobFilter
Expand Down
1 change: 1 addition & 0 deletions engine/app/helpers/good_job/application_helper.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: strict
module GoodJob
module ApplicationHelper
end
Expand Down
1 change: 1 addition & 0 deletions engine/config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: false
GoodJob::Engine.routes.draw do
root to: 'dashboards#index'
resources :active_jobs, only: :show
Expand Down
1 change: 1 addition & 0 deletions engine/lib/good_job/engine.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: strict
module GoodJob
class Engine < ::Rails::Engine
isolate_namespace GoodJob
Expand Down
1 change: 1 addition & 0 deletions good_job.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Gem::Specification.new do |spec|
spec.add_dependency "activerecord", ">= 5.2.0"
spec.add_dependency "concurrent-ruby", ">= 1.0.2"
spec.add_dependency "railties", ">= 5.2.0"
spec.add_dependency "sorbet-runtime-stub"
spec.add_dependency "thor", ">= 0.14.1"
spec.add_dependency "zeitwerk", ">= 2.0"

Expand Down
1 change: 1 addition & 0 deletions lib/active_job/queue_adapters/good_job_adapter.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: true
module ActiveJob # :nodoc:
module QueueAdapters # :nodoc:
# See {GoodJob::Adapter} for details.
Expand Down
1 change: 1 addition & 0 deletions lib/generators/good_job/install_generator.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# typed: true
require 'rails/generators'
require 'rails/generators/active_record'

Expand Down
34 changes: 22 additions & 12 deletions lib/good_job.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
# typed: true
require "sorbet-runtime-stub" unless defined?(T)

require "rails"
require "active_job"
require "active_job/queue_adapters"
Expand Down Expand Up @@ -30,7 +33,7 @@ module GoodJob
# @!scope class
# The logger used by GoodJob (default: +Rails.logger+).
# Use this to redirect logs to a special location or file.
# @return [Logger]
# @return [Logger, nil]
# @example Output GoodJob logs to a file:
# GoodJob.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new("log/my_logs.log"))
mattr_accessor :logger, default: ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new($stdout))
Expand All @@ -42,7 +45,7 @@ module GoodJob
# If you want to preserve jobs for latter inspection, set this to +true+.
# If you want to preserve only jobs that finished with error for latter inspection, set this to +:on_unhandled_error+.
# If +true+, you will need to clean out jobs using the +good_job cleanup_preserved_jobs+ CLI command.
# @return [Boolean]
# @return [Boolean, nil]
mattr_accessor :preserve_job_records, default: false

# @!attribute [rw] retry_on_unhandled_error
Expand All @@ -51,10 +54,11 @@ module GoodJob
# If +true+, causes jobs to be re-queued and retried if they raise an instance of +StandardError+.
# If +false+, jobs will be discarded or marked as finished if they raise an instance of +StandardError+.
# Instances of +Exception+, like +SIGINT+, will *always* be retried, regardless of this attribute's value.
# @return [Boolean]
# @return [Boolean, nil]
mattr_accessor :retry_on_unhandled_error, default: true

# @deprecated Use {GoodJob#retry_on_unhandled_error} instead.
# @return [Boolean, nil]
def self.reperform_jobs_on_standard_error
ActiveSupport::Deprecation.warn(
"Calling 'GoodJob.reperform_jobs_on_standard_error' is deprecated. Please use 'retry_on_unhandled_error'"
Expand All @@ -63,6 +67,8 @@ def self.reperform_jobs_on_standard_error
end

# @deprecated Use {GoodJob#retry_on_unhandled_error=} instead.
# @param value [Boolean]
# @return [Boolean]
def self.reperform_jobs_on_standard_error=(value)
ActiveSupport::Deprecation.warn(
"Setting 'GoodJob.reperform_jobs_on_standard_error=' is deprecated. Please use 'retry_on_unhandled_error='"
Expand All @@ -77,24 +83,25 @@ def self.reperform_jobs_on_standard_error=(value)
# @example Send errors to Sentry
# # config/initializers/good_job.rb
# GoodJob.on_thread_error = -> (exception) { Raven.capture_exception(exception) }
# @return [#call, nil]
# @return [Proc, nil]
mattr_accessor :on_thread_error, default: nil

# Stop executing jobs.
# GoodJob does its work in pools of background threads.
# When forking processes you should shut down these background threads before forking, and restart them after forking.
# For example, you should use +shutdown+ and +restart+ when using async execution mode with Puma.
# See the {file:README.md#executing-jobs-async--in-process} for more explanation and examples.
# @param wait [Boolean] whether to wait for shutdown
# @param timeout [Numeric, nil] Seconds to wait for active threads to finish
# @param wait [Boolean, nil] whether to wait for shutdown
# @return [void]
def self.shutdown(timeout: -1, wait: nil)
timeout = if wait.present?
timeout = if wait.nil?
timeout
else
ActiveSupport::Deprecation.warn(
"Using `GoodJob.shutdown` with `wait:` kwarg is deprecated; use `timeout:` kwarg instead e.g. GoodJob.shutdown(timeout: #{wait ? '-1' : 'nil'})"
)
wait ? -1 : nil
else
timeout
end

executables = Array(Notifier.instances) + Array(Poller.instances) + Array(Scheduler.instances)
Expand All @@ -104,28 +111,31 @@ def self.shutdown(timeout: -1, wait: nil)
# Tests whether jobs have stopped executing.
# @return [Boolean] whether background threads are shut down
def self.shutdown?
Notifier.instances.all?(&:shutdown?) &&
Poller.instances.all?(&:shutdown?) &&
Scheduler.instances.all?(&:shutdown?)
T.must(Notifier.instances).all?(&:shutdown?) &&
T.must(Poller.instances).all?(&:shutdown?) &&
T.must(Scheduler.instances).all?(&:shutdown?)
end

# Stops and restarts executing jobs.
# GoodJob does its work in pools of background threads.
# When forking processes you should shut down these background threads before forking, and restart them after forking.
# For example, you should use +shutdown+ and +restart+ when using async execution mode with Puma.
# See the {file:README.md#executing-jobs-async--in-process} for more explanation and examples.
# @param timeout [Numeric, nil] Seconds to wait for active threads to finish.
# @return [void]
def self.restart(timeout: -1)
executables = Array(Notifier.instances) + Array(Poller.instances) + Array(Scheduler.instances)
_shutdown_all(executables, :restart, timeout: timeout)
end

# Sends +#shutdown+ or +#restart+ to executable objects ({GoodJob::Notifier}, {GoodJob::Poller}, {GoodJob::Scheduler})
# @param executables [Array<(Notifier, Poller, Scheduler)>] Objects to shut down.
# @param executables [Array<Notifier, Poller, Scheduler, MultiScheduler>] Objects to shut down.
# @param method_name [:symbol] Method to call, e.g. +:shutdown+ or +:restart+.
# @param timeout [nil,Numeric]
# @return [void]
def self._shutdown_all(executables, method_name = :shutdown, timeout: -1)
timeout = -1 if timeout.nil?

if timeout.positive?
executables.each { |executable| executable.send(method_name, timeout: nil) }

Expand Down