diff --git a/.rubocop.yml b/.rubocop.yml index f18bd4dc8d..acc1ec6307 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,21 +6,15 @@ AllCops: - tmp/**/* - node_modules/**/* -LineLength: +Metrics/LineLength: Max: 120 -ClassLength: - Enabled: false - -Layout/CommentIndentation: +Metrics/ClassLength: Enabled: false Style/StringLiterals: Enabled: false -HashSyntax: - EnforcedStyle: ruby19 - Style/SignalException: Enabled: false @@ -31,8 +25,9 @@ Layout/SpaceInsideHashLiteralBraces: Lint/AmbiguousOperator: Enabled: false +# Always use `->` Style/Lambda: - Enabled: false + EnforcedStyle: literal Style/SpecialGlobalVars: Enabled: false @@ -55,14 +50,15 @@ Style/StringLiteralsInInterpolation: Style/NumericLiterals: Enabled: false +# prefer simpler `a == 0` over `a.zero?` Style/NumericPredicate: - Enabled: false + EnforcedStyle: comparison Layout/FirstParameterIndentation: Enabled: false Layout/IndentHash: - Enabled: false + EnforcedStyle: consistent Layout/AlignParameters: EnforcedStyle: with_fixed_indentation @@ -92,7 +88,7 @@ Metrics/CyclomaticComplexity: Enabled: false Layout/MultilineMethodCallIndentation: - Enabled: false + Enabled: false # none of the EnforcedStyle options allow for consistent 2-space indent Lint/AmbiguousRegexpLiteral: Enabled: false @@ -103,14 +99,7 @@ Layout/DotPosition: Style/ClassAndModuleChildren: Enabled: false -# %w[] shows that it will return an array -Style/PercentLiteralDelimiters: - PreferredDelimiters: - '%i': '[]' - '%w': '[]' - '%W': '[]' - -# for single `/` more readable +# often single `/` is more readable as `/a\/b/` Style/RegexpLiteral: Enabled: false @@ -138,6 +127,7 @@ Metrics/ParameterLists: Metrics/BlockLength: Enabled: false +# Allow use of `if a = b(1, 2)` for simplified control flow Lint/AssignmentInCondition: Enabled: false diff --git a/app/controllers/csv_exports_controller.rb b/app/controllers/csv_exports_controller.rb index b8281c76e8..7a74ae19fc 100644 --- a/app/controllers/csv_exports_controller.rb +++ b/app/controllers/csv_exports_controller.rb @@ -73,8 +73,8 @@ def user_filter options = {} options[:inherited] = params[:inherited] == "true" options[:deleted] = params[:deleted] == "true" - options[:project_id] = params[:project_id].to_i unless params[:project_id].to_i.zero? - options[:user_id] = params[:user_id].to_i unless params[:user_id].to_i.zero? + options[:project_id] = params[:project_id].to_i unless params[:project_id].to_i == 0 + options[:user_id] = params[:user_id].to_i unless params[:user_id].to_i == 0 options[:datetime] = Time.now.strftime "%Y%m%d_%H%M" options end @@ -118,7 +118,7 @@ def deploy_filter end if project = params[:project].try(:to_i) - if project.positive? + if project > 0 filter['stages.project_id'] = project elsif project.to_s != params[:project] raise "Invalid project id #{params[:project]}" diff --git a/app/controllers/mass_rollouts_controller.rb b/app/controllers/mass_rollouts_controller.rb index 1dd5717a36..6ff71e3aa1 100644 --- a/app/controllers/mass_rollouts_controller.rb +++ b/app/controllers/mass_rollouts_controller.rb @@ -107,7 +107,7 @@ def merge_stage(stage) return "has no template stage to merge into" unless template_stage return "is a template stage" if stage.is_template - return "has no deploy groups" if stage.deploy_groups.count.zero? + return "has no deploy groups" if stage.deploy_groups.count == 0 return "has more than one deploy group" if stage.deploy_groups.count > 1 return "commands in template stage differ" if stage.script != template_stage.script diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 6dfdfc3659..b2043ca6ad 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -311,7 +311,7 @@ def link_to_chart(name, values) max = values.max.round y_axis = [0, max / 4, max / 2, (max / 1.333333).to_i, max].map { |t| duration_text(t) }.join("|") - y_values = values.reverse.map { |v| max.zero? ? max : (v * 100.0 / max).round }.join(",") # values as % of max + y_values = values.reverse.map { |v| max == 0 ? max : (v * 100.0 / max).round }.join(",") # values as % of max params = { cht: "lc", # chart type chtt: name, diff --git a/app/helpers/dashboards_helper.rb b/app/helpers/dashboards_helper.rb index e8aa6be8ef..f41c949cea 100644 --- a/app/helpers/dashboards_helper.rb +++ b/app/helpers/dashboards_helper.rb @@ -6,7 +6,7 @@ def project_has_different_deploys?(deploy_group_versions) def dashboard_project_row_style(project_id) project_versions = @versions[project_id].values.map(&:reference).uniq.count - if project_versions.zero? + if project_versions == 0 'class=no-deploys' elsif project_versions == 1 '' diff --git a/app/helpers/deploys_helper.rb b/app/helpers/deploys_helper.rb index 9cbeb28101..1887f7b0a5 100644 --- a/app/helpers/deploys_helper.rb +++ b/app/helpers/deploys_helper.rb @@ -41,7 +41,7 @@ def file_status_label(status) end def file_changes_label(count, type) - content_tag :span, count.to_s, class: "label #{type}" unless count.zero? + content_tag :span, count.to_s, class: "label #{type}" unless count == 0 end def github_users(github_users) diff --git a/app/models/csv_export.rb b/app/models/csv_export.rb index 622cfa62fb..d2083a438b 100644 --- a/app/models/csv_export.rb +++ b/app/models/csv_export.rb @@ -9,7 +9,7 @@ class CsvExport < ActiveRecord::Base validates :status, inclusion: {in: STATUS_VALUES} delegate :email, to: :user, allow_nil: true - scope :old, lambda { + scope :old, -> { end_date = Rails.application.config.samson.export_job.downloaded_age.seconds.ago timeout_date = Rails.application.config.samson.export_job.max_age.seconds.ago where("(status = 'downloaded' AND updated_at <= :end_date) OR created_at <= :timeout_date", diff --git a/app/models/git_repository.rb b/app/models/git_repository.rb index b7efcb890e..ff2d694dc3 100644 --- a/app/models/git_repository.rb +++ b/app/models/git_repository.rb @@ -106,7 +106,7 @@ def ensure_mirror_current # @returns [block result, false on lock timeout] def exclusive(timeout: 10.minutes) log_wait = proc do |owner| - if Rails.env.test? || (Time.now.to_i % 10).zero? + if Rails.env.test? || (Time.now.to_i % 10) == 0 executor.output.write("Waiting for repository lock for #{owner}\n") end end diff --git a/app/models/image_builder.rb b/app/models/image_builder.rb index b1f01d763f..0bd6ea7847 100644 --- a/app/models/image_builder.rb +++ b/app/models/image_builder.rb @@ -92,7 +92,7 @@ def push_image_to_registries(image_id, build, executor, tag:, override_tag:) digest = nil DockerRegistry.all.each_with_index do |registry, i| - primary = i.zero? + primary = i == 0 repo = build.project.docker_repo(registry, build.dockerfile) if override_tag diff --git a/app/models/job_queue.rb b/app/models/job_queue.rb index 55293c9030..4bed97f700 100644 --- a/app/models/job_queue.rb +++ b/app/models/job_queue.rb @@ -139,7 +139,7 @@ def debug_hash_from_queue(queue) end def staggering_enabled? - ENV['SERVER_MODE'] && !stagger_interval.zero? + ENV['SERVER_MODE'] && stagger_interval != 0 end def stagger_interval diff --git a/lib/samson/boot_check.rb b/lib/samson/boot_check.rb index 33faefab29..5cf473c090 100644 --- a/lib/samson/boot_check.rb +++ b/lib/samson/boot_check.rb @@ -7,7 +7,7 @@ def check # make sure nobody uses connections on the main thread since they will block reloading in dev error = "Do not use AR on the main thread, use ActiveRecord::Base.connection_pool.with_connection" Samson::Retry.until_result tries: 10, wait_time: 0.5, error: error do - ActiveRecord::Base.connection_pool.stat.fetch(:busy).zero? + ActiveRecord::Base.connection_pool.stat.fetch(:busy) == 0 end else # make sure we do not regress into slow startup time by preloading too much diff --git a/lib/samson/logging.rb b/lib/samson/logging.rb index 319aebcc58..1ca7f46d84 100644 --- a/lib/samson/logging.rb +++ b/lib/samson/logging.rb @@ -10,7 +10,7 @@ # log 1 message per request to syslog in json format config.lograge.enabled = true - config.lograge.custom_options = lambda do |event| + config.lograge.custom_options = ->(event) do # show params for every request unwanted_keys = %w[format action controller] params = event.payload[:params].reject { |key, _| unwanted_keys.include? key } diff --git a/lib/samson/retry.rb b/lib/samson/retry.rb index 50daa02d1d..f8b1865d90 100644 --- a/lib/samson/retry.rb +++ b/lib/samson/retry.rb @@ -22,7 +22,7 @@ def self.until_result(tries:, wait_time:, error:) tries -= 1 result = yield return result if result - raise error if tries.zero? + raise error if tries == 0 sleep(wait_time) end end diff --git a/plugins/docker_binary_builder/app/models/binary_builder.rb b/plugins/docker_binary_builder/app/models/binary_builder.rb index c86111c345..8737bb5571 100644 --- a/plugins/docker_binary_builder/app/models/binary_builder.rb +++ b/plugins/docker_binary_builder/app/models/binary_builder.rb @@ -109,7 +109,7 @@ def create_container_options # Mount a cache directory for sharing .m2, .ivy2, .bundler directories between build containers. api_version_major, api_version_minor = docker_api_version.scan(/(\d+)\.(\d+)/).flatten.map(&:to_i) - if api_version_major.zero? || (api_version_major == 1 && api_version_minor <= 14) + if api_version_major == 0 || (api_version_major == 1 && api_version_minor <= 14) fail "Unsupported Docker api version '#{docker_api_version}', use at least v1.15" elsif api_version_major == 1 && api_version_minor <= 22 options.merge!( diff --git a/plugins/jenkins/app/models/samson/jenkins.rb b/plugins/jenkins/app/models/samson/jenkins.rb index 906408d66c..07d3403cf1 100644 --- a/plugins/jenkins/app/models/samson/jenkins.rb +++ b/plugins/jenkins/app/models/samson/jenkins.rb @@ -251,7 +251,7 @@ def notify_emails emails.select! { |e| e.include?('@') } emails.map! { |x| Mail::Address.new(x) } if restricted_domain = ENV["EMAIL_DOMAIN"] - emails.select! { |x| x.domain.casecmp(restricted_domain).zero? } + emails.select! { |x| x.domain.casecmp(restricted_domain) == 0 } end emails.map(&:address).uniq.join(",") end diff --git a/plugins/kubernetes/app/decorators/deploy_group_decorator.rb b/plugins/kubernetes/app/decorators/deploy_group_decorator.rb index 8e6de0cdb4..4116a9cd3c 100644 --- a/plugins/kubernetes/app/decorators/deploy_group_decorator.rb +++ b/plugins/kubernetes/app/decorators/deploy_group_decorator.rb @@ -15,7 +15,7 @@ :cluster_deploy_group, allow_destroy: true, update_only: true, - reject_if: lambda { |h| h[:kubernetes_cluster_id].blank? } + reject_if: ->(h) { h[:kubernetes_cluster_id].blank? } ) def kubernetes_namespace diff --git a/plugins/kubernetes/app/models/kubernetes/api/pod.rb b/plugins/kubernetes/app/models/kubernetes/api/pod.rb index 203eef3570..62636bcebe 100644 --- a/plugins/kubernetes/app/models/kubernetes/api/pod.rb +++ b/plugins/kubernetes/app/models/kubernetes/api/pod.rb @@ -31,7 +31,7 @@ def failed? end def restarted? - @pod.dig(:status, :containerStatuses)&.any? { |s| s.fetch(:restartCount).positive? } + @pod.dig(:status, :containerStatuses)&.any? { |s| s.fetch(:restartCount) > 0 } end def phase diff --git a/plugins/kubernetes/app/models/kubernetes/resource.rb b/plugins/kubernetes/app/models/kubernetes/resource.rb index 13ea9b22f5..b782cf0d47 100644 --- a/plugins/kubernetes/app/models/kubernetes/resource.rb +++ b/plugins/kubernetes/app/models/kubernetes/resource.rb @@ -280,7 +280,7 @@ def request_delete # $ kubectl scale deployment {DEPLOYMENT_NAME} --replicas 0 # "replicas" key is actually removed from "status" map # $ {"status":{"conditions":[...],"observedGeneration":2}} - break if fetch_resource.dig(:status, :replicas).to_i.zero? + break if fetch_resource.dig(:status, :replicas).to_i == 0 end # delete the actual deployment diff --git a/plugins/rollbar_dashboards/test/decorators/project_decorator_test.rb b/plugins/rollbar_dashboards/test/decorators/project_decorator_test.rb index 712a4afe68..bf6a513441 100644 --- a/plugins/rollbar_dashboards/test/decorators/project_decorator_test.rb +++ b/plugins/rollbar_dashboards/test/decorators/project_decorator_test.rb @@ -12,7 +12,7 @@ it "assigns rollbar dashboard attributes" do project.attributes = { rollbar_dashboards_settings_attributes: {0 => {read_token: '123', base_url: 'https://foobar.org'}} - } + } project.rollbar_dashboards_settings.size.must_equal 1 end diff --git a/script/create_many_projects_stages_deploys.rb b/script/create_many_projects_stages_deploys.rb index bd607a2295..4fc48ca0c4 100755 --- a/script/create_many_projects_stages_deploys.rb +++ b/script/create_many_projects_stages_deploys.rb @@ -26,7 +26,7 @@ NUM_PROJECTS.times do |i| project_name = "Project#{i}" - next unless Project.unscoped.where(name: project_name).count.zero? + next unless Project.unscoped.where(name: project_name).count == 0 project = Project.create!(name: project_name, repository_url: "git@github.com:samson-test-org/example-project.git") project.stages.create!(name: "Production", deploy_groups: [pod1, pod2, pod3, pod4, pod5, pod6]) project.stages.create!(name: "Staging", deploy_groups: [pod100, pod101]) diff --git a/test/controllers/integrations/buildkite_controller_test.rb b/test/controllers/integrations/buildkite_controller_test.rb index 9266560fcd..467cfd1d96 100644 --- a/test/controllers/integrations/buildkite_controller_test.rb +++ b/test/controllers/integrations/buildkite_controller_test.rb @@ -51,7 +51,7 @@ end context 'when the buildkite_release_params hook gets trigger' do - let(:buildkite_build_number) { lambda { |_, _| [[:number, 9]] } } + let(:buildkite_build_number) { ->(_, _) { [[:number, 9]] } } before do project.releases.destroy_all project.builds.destroy_all diff --git a/test/models/changeset_test.rb b/test/models/changeset_test.rb index ebc0e989f6..e6e890f4a8 100644 --- a/test/models/changeset_test.rb +++ b/test/models/changeset_test.rb @@ -87,9 +87,7 @@ let(:pr_from_coolcommitter) do Sawyer::Resource.new( sawyer_agent, - user: { - login: 'coolcommitter' - }, + user: {login: 'coolcommitter'}, additions: 10 ) end diff --git a/test/models/multi_lock_test.rb b/test/models/multi_lock_test.rb index 6374ac4315..88d3bca47f 100644 --- a/test/models/multi_lock_test.rb +++ b/test/models/multi_lock_test.rb @@ -21,7 +21,7 @@ def assert_time(operand, number) end it "locks" do - result = MultiLock.lock(1, "a", timeout: 1, failed_to_lock: lambda { |owner| calls << owner }) { calls << 1 } + result = MultiLock.lock(1, "a", timeout: 1, failed_to_lock: ->(owner) { calls << owner }) { calls << 1 } result.must_equal true assert_equal [1], calls end @@ -30,7 +30,7 @@ def assert_time(operand, number) MultiLock.send(:try_lock, 1, "a") assert_time :>, 1 do - result = MultiLock.lock(1, "a", timeout: 1, failed_to_lock: lambda { |owner| calls << owner }) { calls << 1 } + result = MultiLock.lock(1, "a", timeout: 1, failed_to_lock: ->(owner) { calls << owner }) { calls << 1 } result.must_equal false end assert_equal ["a"], calls diff --git a/test/models/release_service_test.rb b/test/models/release_service_test.rb index d841207e56..0ea6c0ae5b 100644 --- a/test/models/release_service_test.rb +++ b/test/models/release_service_test.rb @@ -68,7 +68,7 @@ def assert_failed_ref_find(count) let!(:stage) { project.stages.create!(name: "production", deploy_on_release: true) } it 'does not deploy if the release_deploy_condition check is false' do - deployable_condition_check = lambda { |_, _| false } + deployable_condition_check = ->(_, _) { false } Samson::Hooks.with_callback(:release_deploy_conditions, deployable_condition_check) do |_| service.release(commit: commit, author: author) @@ -78,7 +78,7 @@ def assert_failed_ref_find(count) end it 'does deploy if the release_deploy_condition check is true' do - deployable_condition_check = lambda { |_, _| true } + deployable_condition_check = ->(_, _) { true } Samson::Hooks.with_callback(:release_deploy_conditions, deployable_condition_check) do |_| release = service.release(commit: commit, author: author) diff --git a/test/test_helper.rb b/test/test_helper.rb index 514750104a..cb01fb478c 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -82,7 +82,7 @@ def freeze_time # record hook and their arguments called during a given block def record_hooks(callback, &block) called = [] - Samson::Hooks.with_callback(callback, lambda { |*args| called << args }, &block) + Samson::Hooks.with_callback(callback, ->(*args) { called << args }, &block) called end