From 6ec8ff069ceaa7bb914cbbd97ac248d926ef7e4e Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 24 Mar 2015 18:28:10 -0700 Subject: Enable more rubocop style checks --- .rubocop.yml | 14 +++++++------- app/helpers/gitlab_markdown_helper.rb | 2 +- app/mailers/notify.rb | 2 +- app/models/project_services/asana_service.rb | 2 +- lib/api/helpers.rb | 4 ++-- lib/gitlab/satellite/merge_action.rb | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 53ca2ca2191..7188b0ecefe 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -355,7 +355,7 @@ Style/MultilineBlockChain: Style/MultilineBlockLayout: Description: 'Ensures newlines after multiline block do statements.' - Enabled: false + Enabled: true Style/MultilineIfThen: Description: 'Do not use then for multi-line if/unless.' @@ -390,7 +390,7 @@ Style/NegatedWhile: Style/NestedTernaryOperator: Description: 'Use one expression per branch in a ternary operator.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-nested-ternary' - Enabled: false + Enabled: true Style/Next: Description: 'Use `next` to skip iteration instead of a condition at the end.' @@ -400,17 +400,17 @@ Style/Next: Style/NilComparison: Description: 'Prefer x.nil? to x == nil.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#predicate-methods' - Enabled: false + Enabled: true Style/NonNilCheck: Description: 'Checks for redundant nil checks.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-non-nil-checks' - Enabled: false + Enabled: true Style/Not: Description: 'Use ! instead of not.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#bang-not-not' - Enabled: false + Enabled: true Style/NumericLiterals: Description: >- @@ -424,7 +424,7 @@ Style/OneLineConditional: Favor the ternary operator(?:) over if/then/else/end constructs. StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#ternary-operator' - Enabled: false + Enabled: true Style/OpMethod: Description: 'When defining binary operators, name the argument other.' @@ -436,7 +436,7 @@ Style/ParenthesesAroundCondition: Don't use parentheses around the condition of an if/unless/while. StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-parens-if' - Enabled: false + Enabled: true Style/PercentLiteralDelimiters: Description: 'Use `%`-literal delimiters consistently' diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index f8e104b0827..985def4ad66 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -29,7 +29,7 @@ module GitlabMarkdownHelper end def markdown(text, options={}) - unless (@markdown and options == @options) + unless @markdown && options == @options @options = options gitlab_renderer = Redcarpet::Render::GitlabHTML.new(self, user_color_scheme_class, diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index ee27879cf40..8fcdd3bc853 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -148,7 +148,7 @@ class Notify < ActionMailer::Base headers['References'] = message_id(model) headers['X-GitLab-Project'] = "#{@project.name} | " if @project - if (headers[:subject]) + if headers[:subject] headers[:subject].prepend('Re: ') end diff --git a/app/models/project_services/asana_service.rb b/app/models/project_services/asana_service.rb index d52214cdd69..e6e16058d41 100644 --- a/app/models/project_services/asana_service.rb +++ b/app/models/project_services/asana_service.rb @@ -82,7 +82,7 @@ automatically inspected. Leave blank to include all branches.' branch_restriction = restrict_to_branch.to_s # check the branch restriction is poplulated and branch is not included - if branch_restriction.length > 0 && branch_restriction.index(branch) == nil + if branch_restriction.length > 0 && branch_restriction.index(branch).nil? return end diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index a6e77002a01..be133a2920b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -20,7 +20,7 @@ module API identifier = sudo_identifier() # If the sudo is the current user do nothing - if (identifier && !(@current_user.id == identifier || @current_user.username == identifier)) + if identifier && !(@current_user.id == identifier || @current_user.username == identifier) render_api_error!('403 Forbidden: Must be admin to use sudo', 403) unless @current_user.is_admin? @current_user = User.by_username_or_id(identifier) not_found!("No user id or username for: #{identifier}") if @current_user.nil? @@ -33,7 +33,7 @@ module API identifier ||= params[SUDO_PARAM] ||= env[SUDO_HEADER] # Regex for integers - if (!!(identifier =~ /^[0-9]+$/)) + if !!(identifier =~ /^[0-9]+$/) identifier.to_i else identifier diff --git a/lib/gitlab/satellite/merge_action.rb b/lib/gitlab/satellite/merge_action.rb index 25122666f5e..1f2e5f82dd5 100644 --- a/lib/gitlab/satellite/merge_action.rb +++ b/lib/gitlab/satellite/merge_action.rb @@ -97,7 +97,7 @@ module Gitlab in_locked_and_timed_satellite do |merge_repo| prepare_satellite!(merge_repo) update_satellite_source_and_target!(merge_repo) - if (merge_request.for_fork?) + if merge_request.for_fork? repository = Gitlab::Git::Repository.new(merge_repo.path) commits = Gitlab::Git::Commit.between( repository, -- cgit v1.2.1 From 69454e36f77db6f6e1c45c04c39acf670fe443e4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 24 Mar 2015 18:35:57 -0700 Subject: Style/RedundantReturn enabled --- .rubocop.yml | 2 +- app/helpers/gitlab_markdown_helper.rb | 4 ++-- app/helpers/merge_requests_helper.rb | 2 +- app/helpers/submodule_helper.rb | 7 +++++-- lib/gitlab/git_access.rb | 4 ++-- lib/gitlab/popen.rb | 2 +- lib/gitlab/theme.rb | 2 +- 7 files changed, 13 insertions(+), 10 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 7188b0ecefe..7290d627d24 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -480,7 +480,7 @@ Style/RedundantException: Style/RedundantReturn: Description: "Don't use return where it's not required." StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-explicit-return' - Enabled: false + Enabled: true Style/RedundantSelf: Description: "Don't use self where it's not needed." diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 985def4ad66..08221aaa2f8 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -182,7 +182,7 @@ module GitlabMarkdownHelper def file_exists?(path) return false if path.nil? - return @repository.blob_at(current_sha, path).present? || @repository.tree(current_sha, path).entries.any? + @repository.blob_at(current_sha, path).present? || @repository.tree(current_sha, path).entries.any? end # Check if the path is pointing to a directory(tree) or a file(blob) @@ -190,7 +190,7 @@ module GitlabMarkdownHelper def local_path(path) return "tree" if @repository.tree(current_sha, path).entries.any? return "raw" if @repository.blob_at(current_sha, path).image? - return "blob" + "blob" end def current_sha diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb index 51b60770e0b..54462fd00e3 100644 --- a/app/helpers/merge_requests_helper.rb +++ b/app/helpers/merge_requests_helper.rb @@ -17,7 +17,7 @@ module MergeRequestsHelper end def new_mr_from_push_event(event, target_project) - return { + { merge_request: { source_project_id: event.project.id, target_project_id: target_project.id, diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index 525266fb3b5..241462e5e4c 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -49,7 +49,7 @@ module SubmoduleHelper def standard_links(host, namespace, project, commit) base = [ 'https://', host, '/', namespace, '/', project ].join('') - return base, [ base, '/tree/', commit ].join('') + [base, [ base, '/tree/', commit ].join('')] end def relative_self_links(url, commit) @@ -58,7 +58,10 @@ module SubmoduleHelper else base = [ @project.group.path, '/', url[/([^\/]*)\.git/, 1] ].join('') end - return namespace_project_path(base.namespace, base), + + [ + namespace_project_path(base.namespace, base), namespace_project_tree_path(base.namespace, base, commit) + ] end end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index cb69e4b13d3..573415baec1 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -7,7 +7,7 @@ module Gitlab def self.can_push_to_branch?(user, project, ref) return false unless user - + if project.protected_branch?(ref) && !(project.developers_can_push_to_protected_branch?(ref) && project.team.developer?(user)) user.can?(:push_code_to_protected_branches, project) @@ -83,7 +83,7 @@ module Gitlab end end - return build_status_object(true) + build_status_object(true) end def change_access_check(user, project, change) diff --git a/lib/gitlab/popen.rb b/lib/gitlab/popen.rb index fea4d2d55d2..43e07e09160 100644 --- a/lib/gitlab/popen.rb +++ b/lib/gitlab/popen.rb @@ -29,7 +29,7 @@ module Gitlab @cmd_status = wait_thr.value.exitstatus end - return @cmd_output, @cmd_status + [@cmd_output, @cmd_status] end end end diff --git a/lib/gitlab/theme.rb b/lib/gitlab/theme.rb index 9799e54de5d..43093c7d27e 100644 --- a/lib/gitlab/theme.rb +++ b/lib/gitlab/theme.rb @@ -19,7 +19,7 @@ module Gitlab id ||= Gitlab.config.gitlab.default_theme - return themes[id] + themes[id] end def self.type_css_class_by_id(id) -- cgit v1.2.1