diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index 6fb6b73e093301dfbdf8eee14abf885fc8935cea..83c07bed09125146149ee8dd3e485db05620b688 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -34,16 +34,6 @@ Style/IfUnlessModifier: - 'app/mailers/emails/members.rb' - 'app/mailers/emails/merge_requests.rb' - 'app/models/ability.rb' - - 'app/models/appearance.rb' - - 'app/models/application_setting_implementation.rb' - - 'app/models/bulk_imports/entity.rb' - - 'app/models/ci/application_record.rb' - - 'app/models/ci/build.rb' - - 'app/models/ci/build_trace_chunk.rb' - - 'app/models/ci/job_artifact.rb' - - 'app/models/ci/pending_build.rb' - - 'app/models/ci/pipeline.rb' - - 'app/models/ci/runner.rb' - 'app/models/concerns/ci/artifactable.rb' - 'app/models/concerns/deprecated_assignee.rb' - 'app/models/concerns/group_descendant.rb' diff --git a/app/models/appearance.rb b/app/models/appearance.rb index 35286a9ec94f64c3fe29f2ae646ce5d2d73a6386..2352f6f736c0faeaa86d2341a311b104149eedd8 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -69,9 +69,7 @@ def self.current_without_cache end def single_appearance_row - if self.class.any? - errors.add(:base, _('Only 1 appearances row can exist')) - end + errors.add(:base, _('Only 1 appearances row can exist')) if self.class.any? end def pwa_icon_path_scaled(width) diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index dd972377e8d85b1aa3e001ddd164da542292796f..fe1c75366415f7285ff24dc617a94826793cf672 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -537,7 +537,9 @@ def usage_ping_can_be_configured? def usage_ping_features_enabled return false unless usage_ping_enabled? && super - return include_optional_metrics_in_service_ping if Gitlab.ee? && respond_to?(:include_optional_metrics_in_service_ping) + if Gitlab.ee? && respond_to?(:include_optional_metrics_in_service_ping) + return include_optional_metrics_in_service_ping + end true end @@ -699,14 +701,20 @@ def check_repository_storages_weighted repository_storages_weighted.each do |key, val| next unless val.present? - errors.add(:repository_storages_weighted, _("value for '%{storage}' must be an integer") % { storage: key }) unless val.is_a?(Integer) - errors.add(:repository_storages_weighted, _("value for '%{storage}' must be between 0 and 100") % { storage: key }) unless val.between?(0, 100) + unless val.is_a?(Integer) + errors.add(:repository_storages_weighted, _("value for '%{storage}' must be an integer") % { storage: key }) + end + + unless val.between?(0, 100) + errors.add(:repository_storages_weighted, _("value for '%{storage}' must be between 0 and 100") % { storage: key }) + end end end def check_valid_runner_registrars - valid = valid_runner_registrar_combinations.include?(valid_runner_registrars) - errors.add(:valid_runner_registrars, _("%{value} is not included in the list") % { value: valid_runner_registrars }) unless valid + return if valid_runner_registrar_combinations.include?(valid_runner_registrars) + + errors.add(:valid_runner_registrars, _("%{value} is not included in the list") % { value: valid_runner_registrars }) end def valid_runner_registrar_combinations diff --git a/app/models/bulk_imports/entity.rb b/app/models/bulk_imports/entity.rb index 0c5b3ec0c66f5f25300e69e3acbb1021d74c6f3f..10f55b5454e55c90241a5856e2ef2a1c3968296e 100644 --- a/app/models/bulk_imports/entity.rb +++ b/app/models/bulk_imports/entity.rb @@ -175,9 +175,7 @@ def relation_download_url_path(relation, batch_number = nil) url = File.join(export_relations_url_path_base, 'download') params = { relation: relation } - if batch_number && bulk_import.supports_batched_export? - params.merge!(batched: true, batch_number: batch_number) - end + params.merge!(batched: true, batch_number: batch_number) if batch_number && bulk_import.supports_batched_export? Gitlab::Utils.add_url_parameters(url, params) end @@ -241,9 +239,7 @@ def propagate_cancel private def validate_parent_is_a_group - unless parent.group_entity? - errors.add(:parent, s_('BulkImport|must be a group')) - end + errors.add(:parent, s_('BulkImport|must be a group')) unless parent.group_entity? end def validate_imported_entity_type diff --git a/app/models/ci/application_record.rb b/app/models/ci/application_record.rb index 52f02bfb2fd81f5adad3ae4d41cff78a0e934ca1..f0d9abb656236339c3fb95dd43721c3e57f04a7b 100644 --- a/app/models/ci/application_record.rb +++ b/app/models/ci/application_record.rb @@ -4,9 +4,7 @@ module Ci class ApplicationRecord < ::ApplicationRecord self.abstract_class = true - if Gitlab::Database.has_config?(:ci) - connects_to database: { writing: :ci, reading: :ci } - end + connects_to database: { writing: :ci, reading: :ci } if Gitlab::Database.has_config?(:ci) def self.table_name_prefix 'ci_' diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 774218a0ab9c36a638aaaff7169e407e4fe05a41..b425a079408ea36934616b60c1a36e9264f7b414 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -828,7 +828,9 @@ def artifact_access_setting_in_config artifacts_public = options.dig(:artifacts, :public) artifacts_access = options.dig(:artifacts, :access) - raise ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive' if !artifacts_public.nil? && !artifacts_access.nil? + if !artifacts_public.nil? && !artifacts_access.nil? + raise ArgumentError, 'artifacts:public and artifacts:access are mutually exclusive' + end return :public if artifacts_public == true || artifacts_access == 'all' return :private if artifacts_public == false || artifacts_access == 'developer' @@ -867,9 +869,7 @@ def artifacts_expire_in def artifacts_expire_in=(value) self.artifacts_expire_at = - if value - ChronicDuration.parse(value)&.seconds&.from_now - end + (ChronicDuration.parse(value)&.seconds&.from_now if value) end def has_expired_locked_archive_artifacts? @@ -997,9 +997,7 @@ def hide_secrets(data, metrics = ::Gitlab::Ci::Trace::Metrics.new) Gitlab::Ci::MaskSecret.mask!(trace, project.runners_token) if project Gitlab::Ci::MaskSecret.mask!(trace, token) if token - if trace != data - metrics.increment_trace_operation(operation: :mutated) - end + metrics.increment_trace_operation(operation: :mutated) if trace != data end end diff --git a/app/models/ci/build_trace_chunk.rb b/app/models/ci/build_trace_chunk.rb index 10b19a83c7135bfcf0b445cc21916e49716be4a7..54cb217315480a798f970432c6fe33bf81d3438f 100644 --- a/app/models/ci/build_trace_chunk.rb +++ b/app/models/ci/build_trace_chunk.rb @@ -275,9 +275,7 @@ def unsafe_set_data!(value) def unsafe_append_data!(value, offset) new_size = value.bytesize + offset - if new_size > CHUNK_SIZE - raise ArgumentError, 'New data size exceeds chunk size' - end + raise ArgumentError, 'New data size exceeds chunk size' if new_size > CHUNK_SIZE current_store.append_data(self, value, offset).then do |stored| metrics.increment_trace_operation(operation: :appended) diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index f50468c1678f31aaecdf73c0d68357167c9439a9..9cbb9f6fa47d3f06ace8039220365912a06b1d45 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -179,9 +179,7 @@ def expire_in def expire_in=(value) self.expire_at = - if value - ::Gitlab::Ci::Build::DurationParser.new(value).seconds_from_now - end + (::Gitlab::Ci::Build::DurationParser.new(value).seconds_from_now if value) end def stored? diff --git a/app/models/ci/pending_build.rb b/app/models/ci/pending_build.rb index 762f6d5917d3d9b38bcd92183794ec4dc790394f..316a780a0a43038eb8d3d68e7bb1473ed80999c1 100644 --- a/app/models/ci/pending_build.rb +++ b/app/models/ci/pending_build.rb @@ -58,9 +58,7 @@ def args_from_build(build) instance_runners_enabled: shared_runners_enabled?(project) } - if group_runners_enabled?(project) - args.store(:namespace_traversal_ids, project.namespace.traversal_ids) - end + args.store(:namespace_traversal_ids, project.namespace.traversal_ids) if group_runners_enabled?(project) args end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 13c981000261458242efbe2381ec43df04d028cd..a05cb9877f30513760600c910af26ada6e3caba2 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -636,9 +636,7 @@ def triggered_pipelines_with_preloads end def valid_commit_sha - if Gitlab::Git.blank_ref?(self.sha) - self.errors.add(:sha, "can't be 00000000 (branch removal)") - end + self.errors.add(:sha, "can't be 00000000 (branch removal)") if Gitlab::Git.blank_ref?(self.sha) end def git_author_name @@ -751,9 +749,7 @@ def retried def coverage coverage_array = latest_statuses.map(&:coverage).compact - if coverage_array.size >= 1 - coverage_array.sum / coverage_array.size - end + coverage_array.sum / coverage_array.size if coverage_array.size >= 1 end def update_builds_coverage @@ -1396,9 +1392,7 @@ def has_test_reports? def age_in_minutes return 0 unless persisted? - unless has_attribute?(:created_at) - raise ArgumentError, 'pipeline not fully loaded' - end + raise ArgumentError, 'pipeline not fully loaded' unless has_attribute?(:created_at) return 0 unless created_at diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb index 19ecc411560d95e660bc29c7d567e0f073011bf1..6763afdb3a15c707402a0cbb4af1ddd068502719 100644 --- a/app/models/ci/runner.rb +++ b/app/models/ci/runner.rb @@ -582,39 +582,27 @@ def tag_constraints end def no_sharding_key_id - if sharding_key_id - errors.add(:runner, 'cannot have sharding_key_id assigned') - end + errors.add(:runner, 'cannot have sharding_key_id assigned') if sharding_key_id end def no_projects - if runner_projects.any? - errors.add(:runner, 'cannot have projects assigned') - end + errors.add(:runner, 'cannot have projects assigned') if runner_projects.any? end def no_groups - if runner_namespaces.any? - errors.add(:runner, 'cannot have groups assigned') - end + errors.add(:runner, 'cannot have groups assigned') if runner_namespaces.any? end def any_project - unless runner_projects.any? - errors.add(:runner, 'needs to be assigned to at least one project') - end + errors.add(:runner, 'needs to be assigned to at least one project') unless runner_projects.any? end def exactly_one_group - unless runner_namespaces.size == 1 - errors.add(:runner, 'needs to be assigned to exactly one group') - end + errors.add(:runner, 'needs to be assigned to exactly one group') unless runner_namespaces.size == 1 end def no_allowed_plan_ids - unless allowed_plan_ids.empty? - errors.add(:runner, 'cannot have allowed plans assigned') - end + errors.add(:runner, 'cannot have allowed plans assigned') unless allowed_plan_ids.empty? end def partition_id_prefix_in_16_bit_encode