diff --git a/.rubocop.yml b/.rubocop.yml index b4ca11c8343..89aa0591c31 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -76,7 +76,7 @@ Style/BlockEndNewline: Description: 'Put end statement of multiline block on its own line.' Enabled: true -Style/Blocks: +Style/BlockDelimiters: Description: >- Avoid using {...} for multi-line blocks (multiline chaining is always ugly). @@ -232,6 +232,10 @@ Style/EvenOdd: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#predicate-methods' Enabled: false +Style/ExtraSpacing: + Description: 'Do not use unnecessary spacing.' + Enabled: false + Style/FileName: Description: 'Use snake_case for source file names.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#snake-case-files' @@ -431,6 +435,14 @@ Style/OpMethod: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#other-arg' Enabled: false +Style/ParallelAssignment: + Description: >- + Check for simple usages of parallel assignment. + It will only warn when the number of variables + matches on both sides of the assignment. + StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#parallel-assignment' + Enabled: false + Style/ParenthesesAroundCondition: Description: >- Don't use parentheses around the condition of an @@ -669,6 +681,13 @@ Style/TrailingWhitespace: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#no-trailing-whitespace' Enabled: false +Style/TrailingUnderscoreVariable: + Description: >- + Checks for the usage of unneeded trailing underscores at the + end of parallel variable assignment. + AllowNamedUnderscoreVariables: true + Enabled: false + Style/TrivialAccessors: Description: 'Prefer attr_* methods to trivial readers/writers.' StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#attr_family' @@ -690,11 +709,6 @@ Style/UnneededPercentQ: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#percent-q' Enabled: false -Style/UnneededPercentX: - Description: 'Checks for %x when `` would do.' - StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#percent-x' - Enabled: false - Style/VariableInterpolation: Description: >- Don't interpolate global, instance and class variables @@ -778,6 +792,10 @@ Metrics/MethodLength: StyleGuide: 'https://github.com/bbatsov/ruby-style-guide#short-methods' Enabled: false +Metrics/ModuleLength: + Description: 'Avoid modules longer than 100 lines of code.' + Enabled: false + #################### Lint ################################ ### Warnings @@ -961,6 +979,12 @@ Rails/ActionFilter: Description: 'Enforces consistent use of action filter methods.' Enabled: true +Rails/Date: + Description: >- + Checks the correct usage of date aware methods, + such as Date.today, Date.current etc. + Enabled: false + Rails/DefaultScope: Description: 'Checks if the argument passed to default_scope is a block.' Enabled: false @@ -987,6 +1011,12 @@ Rails/ScopeArgs: Description: 'Checks the arguments of ActiveRecord scopes.' Enabled: false +Rails/TimeZone: + Description: 'Checks the correct usage of time zone aware methods.' + StyleGuide: 'https://github.com/bbatsov/rails-style-guide#time' + Reference: 'http://danilenko.org/2012/7/6/rails_timezones' + Enabled: false + Rails/Validation: Description: 'Use validates :attribute, hash of validations.' Enabled: false diff --git a/Gemfile b/Gemfile index 7298e21ce66..9131797ed15 100644 --- a/Gemfile +++ b/Gemfile @@ -261,7 +261,7 @@ group :development, :test do gem 'spring-commands-spinach', '~> 1.0.0' gem 'spring-commands-teaspoon', '~> 0.0.2' - gem 'rubocop', '~> 0.28.0', require: false + gem 'rubocop', '~> 0.35.0', require: false gem 'coveralls', '~> 0.8.2', require: false gem 'simplecov', '~> 0.10.0', require: false gem 'flog', require: false diff --git a/Gemfile.lock b/Gemfile.lock index ff57460f5bb..796c90573cc 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -513,7 +513,7 @@ GEM multi_json (~> 1.0) websocket-driver (>= 0.2.0) posix-spawn (0.3.11) - powerpack (0.0.9) + powerpack (0.1.1) pry (0.10.3) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -637,12 +637,13 @@ GEM rspec-mocks (~> 3.3.0) rspec-support (~> 3.3.0) rspec-support (3.3.0) - rubocop (0.28.0) + rubocop (0.35.1) astrolabe (~> 1.3) - parser (>= 2.2.0.pre.7, < 3.0) - powerpack (~> 0.0.6) + parser (>= 2.2.3.0, < 3.0) + powerpack (~> 0.1) rainbow (>= 1.99.1, < 3.0) - ruby-progressbar (~> 1.4) + ruby-progressbar (~> 1.7) + tins (<= 1.6.0) ruby-fogbugz (0.2.1) crack (~> 0.4) ruby-progressbar (1.7.5) @@ -945,7 +946,7 @@ DEPENDENCIES rouge (~> 1.10.1) rqrcode-rails3 (~> 0.1.7) rspec-rails (~> 3.3.0) - rubocop (~> 0.28.0) + rubocop (~> 0.35.0) ruby-fogbugz (~> 0.2.1) sanitize (~> 2.0) sass-rails (~> 4.0.5) diff --git a/app/controllers/dashboard/snippets_controller.rb b/app/controllers/dashboard/snippets_controller.rb index f4354c6d8ca..b3594d82530 100644 --- a/app/controllers/dashboard/snippets_controller.rb +++ b/app/controllers/dashboard/snippets_controller.rb @@ -1,6 +1,7 @@ class Dashboard::SnippetsController < Dashboard::ApplicationController def index - @snippets = SnippetsFinder.new.execute(current_user, + @snippets = SnippetsFinder.new.execute( + current_user, filter: :by_user, user: current_user, scope: params[:scope] diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index ae6e9f6fd38..ee705f32e81 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -69,7 +69,7 @@ class Projects::NotesController < Projects::ApplicationController data = { author: current_user, is_award: true, - note: note_params[:note].gsub(":", '') + note: note_params[:note].delete(":") } note = noteable.notes.find_by(data) diff --git a/app/controllers/projects/protected_branches_controller.rb b/app/controllers/projects/protected_branches_controller.rb index 6b52eccebf7..e49259c34b6 100644 --- a/app/controllers/projects/protected_branches_controller.rb +++ b/app/controllers/projects/protected_branches_controller.rb @@ -21,7 +21,7 @@ class Projects::ProtectedBranchesController < Projects::ApplicationController if protected_branch && protected_branch.update_attributes( - developers_can_push: params[:developers_can_push] + developers_can_push: params[:developers_can_push] ) respond_to do |format| diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 21f962df206..e3bafce6910 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -61,7 +61,7 @@ module ApplicationHelper options[:class] ||= '' options[:class] << ' identicon' bg_key = project.id % 7 - style = "background-color: ##{ allowed_colors.values[bg_key] }; color: #555" + style = "background-color: ##{allowed_colors.values[bg_key]}; color: #555" content_tag(:div, class: options[:class], style: style) do project.name[0, 1].upcase diff --git a/app/helpers/external_wiki_helper.rb b/app/helpers/external_wiki_helper.rb index 838b85afdfe..1f3401f2906 100644 --- a/app/helpers/external_wiki_helper.rb +++ b/app/helpers/external_wiki_helper.rb @@ -1,7 +1,7 @@ module ExternalWikiHelper def get_project_wiki_path(project) external_wiki_service = project.services. - select { |service| service.to_param == 'external_wiki' }.first + find { |service| service.to_param == 'external_wiki' } if external_wiki_service.present? && external_wiki_service.active? external_wiki_service.properties['external_wiki_url'] else diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index a0cf3dc0843..ca41657cec1 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -65,7 +65,8 @@ module GitlabMarkdownHelper end def asciidoc(text) - Gitlab::Asciidoc.render(text, + Gitlab::Asciidoc.render( + text, project: @project, current_user: (current_user if defined?(current_user)), diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d061136b7b8..777817e24aa 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -330,10 +330,9 @@ module ProjectsHelper def filename_path(project, filename) if project && blob = project.repository.send(filename) namespace_project_blob_path( - project.namespace, - project, - tree_join(project.default_branch, - blob.name) + project.namespace, + project, + tree_join(project.default_branch, blob.name) ) end end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 886a1e734b5..f448dd0ab61 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -79,7 +79,7 @@ module TreeHelper part_path = File.join(part_path, part) unless part_path.empty? part_path = part if part_path.empty? - next unless parts.last(2).include?(part) if parts.count > max_links + next if parts.count > max_links && !parts.last(2).include?(part) yield(part, tree_join(@ref, part_path)) end end diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index 9beb0de68f3..3bbdd9cee76 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -17,7 +17,7 @@ class Notify < BaseMailer subject: subject, body: body.html_safe, content_type: 'text/html' - ) + ) end # Splits "gitlab.corp.company.com" up into "gitlab.corp.company.com", diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index faa0bdf840b..1f4e8b3ef24 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -126,12 +126,12 @@ class ApplicationSetting < ActiveRecord::Base def restricted_signup_domains_raw=(values) self.restricted_signup_domains = [] self.restricted_signup_domains = values.split( - /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace - | # or - \s # any whitespace character - | # or - [\r\n] # any number of newline characters - /x) + /\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace + | # or + \s # any whitespace character + | # or + [\r\n] # any number of newline characters + /x) self.restricted_signup_domains.reject! { |d| d.empty? } end end diff --git a/app/models/concerns/token_authenticatable.rb b/app/models/concerns/token_authenticatable.rb index 56d38fe8250..488ff8c31b7 100644 --- a/app/models/concerns/token_authenticatable.rb +++ b/app/models/concerns/token_authenticatable.rb @@ -13,7 +13,7 @@ module TokenAuthenticatable @token_fields << token_field define_singleton_method("find_by_#{token_field}") do |token| - where(token_field => token).first if token + find_by(token_field => token) if token end define_method("ensure_#{token_field}") do @@ -37,7 +37,7 @@ module TokenAuthenticatable def generate_token_for(token_field) loop do token = Devise.friendly_token - break token unless self.class.unscoped.where(token_field => token).first + break token unless self.class.unscoped.find_by(token_field => token) end end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index f6f77a16267..d7430d36c41 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -194,9 +194,7 @@ class MergeRequest < ActiveRecord::Base similar_mrs = similar_mrs.where('id not in (?)', self.id) if self.id if similar_mrs.any? errors.add :validate_branches, - "Cannot Create: This merge request already exists: #{ - similar_mrs.pluck(:title) - }" + "Cannot Create: This merge request already exists: #{similar_mrs.pluck(:title)}" end end end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 1c4e101cc10..adafabbec07 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -45,7 +45,7 @@ class Namespace < ActiveRecord::Base class << self def by_path(path) - where('lower(path) = :value', value: path.downcase).first + find_by('lower(path) = :value', value: path.downcase) end # Case insensetive search for namespace by path or name @@ -148,6 +148,6 @@ class Namespace < ActiveRecord::Base end def find_fork_of(project) - projects.joins(:forked_project_link).where('forked_project_links.forked_from_project_id = ?', project.id).first + projects.joins(:forked_project_link).find_by('forked_project_links.forked_from_project_id = ?', project.id) end end diff --git a/app/models/project.rb b/app/models/project.rb index e1f7bf971e3..87116451caa 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -265,7 +265,7 @@ class Project < ActiveRecord::Base joins(:namespace). iwhere('namespaces.path' => namespace_path) - projects.where('projects.path' => project_path).take || + projects.find_by('projects.path' => project_path) || projects.iwhere('projects.path' => project_path).take end @@ -450,7 +450,7 @@ class Project < ActiveRecord::Base end def external_issue_tracker - @external_issues_tracker ||= external_issues_trackers.select(&:activated?).first + @external_issues_tracker ||= external_issues_trackers.find(&:activated?) end def can_have_issues_tracker_id? @@ -496,7 +496,7 @@ class Project < ActiveRecord::Base end def ci_service - @ci_service ||= ci_services.select(&:activated?).first + @ci_service ||= ci_services.find(&:activated?) end def avatar_type @@ -547,7 +547,7 @@ class Project < ActiveRecord::Base end def project_member_by_name_or_email(name = nil, email = nil) - user = users.where('name like ? or email like ?', name, email).first + user = users.find_by('name like ? or email like ?', name, email) project_members.where(user: user) if user end @@ -722,7 +722,7 @@ class Project < ActiveRecord::Base end def project_member(user) - project_members.where(user_id: user).first + project_members.find_by(user_id: user) end def default_branch diff --git a/app/models/project_services/bamboo_service.rb b/app/models/project_services/bamboo_service.rb index 0a61ad96a0e..aa8746beb80 100644 --- a/app/models/project_services/bamboo_service.rb +++ b/app/models/project_services/bamboo_service.rb @@ -27,12 +27,10 @@ class BambooService < CiService validates :build_key, presence: true, if: :activated? validates :username, presence: true, - if: ->(service) { service.password? }, - if: :activated? + if: ->(service) { service.activated? && service.password } validates :password, presence: true, - if: ->(service) { service.username? }, - if: :activated? + if: ->(service) { service.activated? && service.username } attr_accessor :response diff --git a/app/models/project_services/flowdock_service.rb b/app/models/project_services/flowdock_service.rb index 27fc19379f1..15c7c907f7e 100644 --- a/app/models/project_services/flowdock_service.rb +++ b/app/models/project_services/flowdock_service.rb @@ -58,6 +58,6 @@ class FlowdockService < Service repo_url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}", commit_url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/commit/%s", diff_url: "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/compare/%s...%s", - ) + ) end end diff --git a/app/models/project_services/gemnasium_service.rb b/app/models/project_services/gemnasium_service.rb index 91ef267ad79..202fee042e3 100644 --- a/app/models/project_services/gemnasium_service.rb +++ b/app/models/project_services/gemnasium_service.rb @@ -57,6 +57,6 @@ class GemnasiumService < Service token: token, api_key: api_key, repo: project.repository.path_to_repo - ) + ) end end diff --git a/app/models/project_services/teamcity_service.rb b/app/models/project_services/teamcity_service.rb index 29d4236745a..a63700693d7 100644 --- a/app/models/project_services/teamcity_service.rb +++ b/app/models/project_services/teamcity_service.rb @@ -27,12 +27,10 @@ class TeamcityService < CiService validates :build_type, presence: true, if: :activated? validates :username, presence: true, - if: ->(service) { service.password? }, - if: :activated? + if: ->(service) { service.activated? && service.password } validates :password, presence: true, - if: ->(service) { service.username? }, - if: :activated? + if: ->(service) { service.activated? && service.username } attr_accessor :response @@ -147,6 +145,6 @@ class TeamcityService < CiService '', headers: { 'Content-type' => 'application/xml' }, basic_auth: auth - ) + ) end end diff --git a/app/models/user.rb b/app/models/user.rb index fdd14f4571d..e0ce091c54e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -220,9 +220,9 @@ class User < ActiveRecord::Base def find_for_database_authentication(warden_conditions) conditions = warden_conditions.dup if login = conditions.delete(:login) - where(conditions).where(["lower(username) = :value OR lower(email) = :value", { value: login.downcase }]).first + where(conditions).find_by("lower(username) = :value OR lower(email) = :value", value: login.downcase) else - where(conditions).first + find_by(conditions) end end @@ -285,7 +285,7 @@ class User < ActiveRecord::Base end def by_username_or_id(name_or_id) - where('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i).first + find_by('users.username = ? OR users.id = ?', name_or_id.to_s, name_or_id.to_i) end def build_user(attrs = {}) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index b26c7513f5b..8b3d56c2b4c 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -112,7 +112,7 @@ module MergeRequests merge_requests_for_source_branch.each do |merge_request| SystemNoteService.change_branch_presence( - merge_request, merge_request.project, @current_user, + merge_request, merge_request.project, @current_user, :source, @branch_name, presence) end end diff --git a/config/initializers/carrierwave.rb b/config/initializers/carrierwave.rb index bfb8656df55..df28d30d750 100644 --- a/config/initializers/carrierwave.rb +++ b/config/initializers/carrierwave.rb @@ -31,11 +31,11 @@ if File.exists?(aws_file) if Rails.env.test? Fog.mock! connection = ::Fog::Storage.new( - aws_access_key_id: AWS_CONFIG['access_key_id'], - aws_secret_access_key: AWS_CONFIG['secret_access_key'], - provider: 'AWS', - region: AWS_CONFIG['region'] - ) + aws_access_key_id: AWS_CONFIG['access_key_id'], + aws_secret_access_key: AWS_CONFIG['secret_access_key'], + provider: 'AWS', + region: AWS_CONFIG['region'] + ) connection.directories.create(key: AWS_CONFIG['bucket']) end end diff --git a/config/routes.rb b/config/routes.rb index e2d4fcb65a8..57be57e3251 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -441,7 +441,7 @@ Rails.application.routes.draw do scope do post( - '/create_dir/*id', + '/create_dir/*id', to: 'tree#create_dir', constraints: { id: /.+/ }, as: 'create_dir' diff --git a/features/steps/explore/groups.rb b/features/steps/explore/groups.rb index 87cd33c37eb..87f32e70d59 100644 --- a/features/steps/explore/groups.rb +++ b/features/steps/explore/groups.rb @@ -75,18 +75,18 @@ class Spinach::Features::ExploreGroups < Spinach::FeatureSteps name: projectname, path: "#{groupname}-#{projectname}", visibility_level: visibility_level - ) + ) create(:issue, title: "#{projectname} feature", project: project - ) + ) create(:merge_request, title: "#{projectname} feature implemented", source_project: project, target_project: project - ) + ) create(:closed_issue_event, project: project - ) + ) end end diff --git a/features/steps/explore/projects.rb b/features/steps/explore/projects.rb index f819dec2192..742ba5d71f6 100644 --- a/features/steps/explore/projects.rb +++ b/features/steps/explore/projects.rb @@ -61,11 +61,11 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps create(:issue, title: "Bug", project: public_project - ) + ) create(:issue, title: "New feature", project: public_project - ) + ) visit namespace_project_issues_path(public_project.namespace, public_project) end @@ -80,11 +80,11 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps create(:issue, title: "Internal Bug", project: internal_project - ) + ) create(:issue, title: "New internal feature", project: internal_project - ) + ) visit namespace_project_issues_path(internal_project.namespace, internal_project) end @@ -104,7 +104,7 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps title: "Bug fix for public project", source_project: public_project, target_project: public_project, - ) + ) end step 'I should see list of merge requests for "Community" project' do @@ -121,7 +121,7 @@ class Spinach::Features::ExploreProjects < Spinach::FeatureSteps title: "Feature implemented", source_project: internal_project, target_project: internal_project - ) + ) end step 'I should see list of merge requests for "Internal" project' do diff --git a/features/steps/groups.rb b/features/steps/groups.rb index f5e3fee61c0..4c5122d1b7d 100644 --- a/features/steps/groups.rb +++ b/features/steps/groups.rb @@ -85,7 +85,7 @@ class Spinach::Features::Groups < Spinach::FeatureSteps step 'I should see new group "Owned" avatar' do expect(owned_group.avatar).to be_instance_of AvatarUploader - expect(owned_group.avatar.url).to eq "/uploads/group/avatar/#{ Group.find_by(name:"Owned").id }/banana_sample.gif" + expect(owned_group.avatar.url).to eq "/uploads/group/avatar/#{Group.find_by(name:"Owned").id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/profile/profile.rb b/features/steps/profile/profile.rb index 40b2aa7c357..0305f7e6da0 100644 --- a/features/steps/profile/profile.rb +++ b/features/steps/profile/profile.rb @@ -34,7 +34,7 @@ class Spinach::Features::Profile < Spinach::FeatureSteps step 'I should see new avatar' do expect(@user.avatar).to be_instance_of AvatarUploader - expect(@user.avatar.url).to eq "/uploads/user/avatar/#{ @user.id }/banana_sample.gif" + expect(@user.avatar.url).to eq "/uploads/user/avatar/#{@user.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/project/project.rb b/features/steps/project/project.rb index 9ca7c8ebbc7..37bf52b4a95 100644 --- a/features/steps/project/project.rb +++ b/features/steps/project/project.rb @@ -37,7 +37,7 @@ class Spinach::Features::Project < Spinach::FeatureSteps step 'I should see new project avatar' do expect(@project.avatar).to be_instance_of AvatarUploader url = @project.avatar.url - expect(url).to eq "/uploads/project/avatar/#{ @project.id }/banana_sample.gif" + expect(url).to eq "/uploads/project/avatar/#{@project.id}/banana_sample.gif" end step 'I should see the "Remove avatar" button' do diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index b88709620ab..0c6df18ce2e 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -238,13 +238,13 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I am redirected to the new file' do - expect(current_path).to eq(namespace_project_blob_path( - @project.namespace, @project, 'master/' + new_file_name)) + expect(current_path).to eq( + namespace_project_blob_path(@project.namespace, @project, 'master/' + new_file_name)) end step 'I am redirected to the new file with directory' do - expect(current_path).to eq(namespace_project_blob_path( - @project.namespace, @project, 'master/' + new_file_name_with_directory)) + expect(current_path).to eq( + namespace_project_blob_path(@project.namespace, @project, 'master/' + new_file_name_with_directory)) end step 'I am redirected to the new merge request page' do @@ -252,8 +252,8 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I am redirected to the root directory' do - expect(current_path).to eq(namespace_project_tree_path( - @project.namespace, @project, 'master/')) + expect(current_path).to eq( + namespace_project_tree_path(@project.namespace, @project, 'master/')) end step "I don't see the permalink link" do diff --git a/features/steps/shared/paths.rb b/features/steps/shared/paths.rb index c74a5fd3bc7..b33bd332655 100644 --- a/features/steps/shared/paths.rb +++ b/features/steps/shared/paths.rb @@ -212,8 +212,8 @@ module SharedPaths end step 'I visit a binary file in the repo' do - visit namespace_project_blob_path(@project.namespace, @project, File.join( - root_ref, 'files/images/logo-black.png')) + visit namespace_project_blob_path(@project.namespace, @project, + File.join(root_ref, 'files/images/logo-black.png')) end step "I visit my project's commits page" do @@ -316,8 +316,8 @@ module SharedPaths end step 'I am on the ".gitignore" edit file page' do - expect(current_path).to eq(namespace_project_edit_blob_path( - @project.namespace, @project, File.join(root_ref, '.gitignore'))) + expect(current_path).to eq( + namespace_project_edit_blob_path(@project.namespace, @project, File.join(root_ref, '.gitignore'))) end step 'I visit project source page for "6d39438"' do diff --git a/lib/api/entities.rb b/lib/api/entities.rb index b1cd80bdf65..a5daa45faf0 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -67,7 +67,7 @@ module API expose :shared_runners_enabled expose :creator_id expose :namespace - expose :forked_from_project, using: Entities::ForkedFromProject, if: lambda{ | project, options | project.forked? } + expose :forked_from_project, using: Entities::ForkedFromProject, if: lambda{ |project, options| project.forked? } expose :avatar_url expose :star_count, :forks_count end diff --git a/lib/banzai/filter/markdown_filter.rb b/lib/banzai/filter/markdown_filter.rb index 0072bab1f99..d09cf41df39 100644 --- a/lib/banzai/filter/markdown_filter.rb +++ b/lib/banzai/filter/markdown_filter.rb @@ -6,7 +6,7 @@ module Banzai class MarkdownFilter < HTML::Pipeline::TextFilter def initialize(text, context = nil, result = nil) super text, context, result - @text = @text.gsub "\r", '' + @text = @text.delete "\r" end def call diff --git a/lib/banzai/filter/table_of_contents_filter.rb b/lib/banzai/filter/table_of_contents_filter.rb index 92d130074dc..9b3e67206d5 100644 --- a/lib/banzai/filter/table_of_contents_filter.rb +++ b/lib/banzai/filter/table_of_contents_filter.rb @@ -31,7 +31,7 @@ module Banzai id = text.downcase id.gsub!(PUNCTUATION_REGEXP, '') # remove punctuation - id.gsub!(' ', '-') # replace spaces with dash + id.tr!(' ', '-') # replace spaces with dash id.squeeze!('-') # replace multiple dashes with one uniq = (headers[id] > 0) ? "-#{headers[id]}" : '' diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index 87ac30b5ffe..459e3d6bcdb 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -2,7 +2,7 @@ module Gitlab class Shell class Error < StandardError; end - class KeyAdder < Struct.new(:io) + KeyAdder = Struct.new(:io) do def add_key(id, key) key.gsub!(/[[:space:]]+/, ' ').strip! io.puts("#{id}\t#{key}") diff --git a/lib/gitlab/bitbucket_import/project_creator.rb b/lib/gitlab/bitbucket_import/project_creator.rb index 35e34d033e0..03aac1a025a 100644 --- a/lib/gitlab/bitbucket_import/project_creator.rb +++ b/lib/gitlab/bitbucket_import/project_creator.rb @@ -11,7 +11,8 @@ module Gitlab end def execute - project = ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new( + current_user, name: repo["name"], path: repo["slug"], description: repo["description"], diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 142058aa69d..79061cd0141 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -46,11 +46,11 @@ module Gitlab end def added_lines - diff_lines.select(&:added?).size + diff_lines.count(&:added?) end def removed_lines - diff_lines.select(&:removed?).size + diff_lines.count(&:removed?) end end end diff --git a/lib/gitlab/fogbugz_import/importer.rb b/lib/gitlab/fogbugz_import/importer.rb index 496256700b8..403ebeec474 100644 --- a/lib/gitlab/fogbugz_import/importer.rb +++ b/lib/gitlab/fogbugz_import/importer.rb @@ -199,7 +199,7 @@ module Gitlab s = s.gsub(/^#/, "\\#") s = s.gsub(/^-/, "\\-") s = s.gsub("`", "\\~") - s = s.gsub("\r", "") + s = s.delete("\r") s = s.gsub("\n", " \n") s end diff --git a/lib/gitlab/fogbugz_import/project_creator.rb b/lib/gitlab/fogbugz_import/project_creator.rb index 8b1b6f48ed5..e0163499e30 100644 --- a/lib/gitlab/fogbugz_import/project_creator.rb +++ b/lib/gitlab/fogbugz_import/project_creator.rb @@ -12,7 +12,8 @@ module Gitlab end def execute - project = ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new( + current_user, name: repo.safe_name, path: repo.path, namespace: namespace, diff --git a/lib/gitlab/gitlab_import/project_creator.rb b/lib/gitlab/gitlab_import/project_creator.rb index d9452de6a50..7baaadb813c 100644 --- a/lib/gitlab/gitlab_import/project_creator.rb +++ b/lib/gitlab/gitlab_import/project_creator.rb @@ -11,7 +11,8 @@ module Gitlab end def execute - project = ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new( + current_user, name: repo["name"], path: repo["path"], description: repo["description"], diff --git a/lib/gitlab/gitorious_import/project_creator.rb b/lib/gitlab/gitorious_import/project_creator.rb index cc9a91c91f4..8e22aa9286d 100644 --- a/lib/gitlab/gitorious_import/project_creator.rb +++ b/lib/gitlab/gitorious_import/project_creator.rb @@ -10,7 +10,8 @@ module Gitlab end def execute - ::Projects::CreateService.new(current_user, + ::Projects::CreateService.new( + current_user, name: repo.name, path: repo.path, description: repo.description, diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 87fee28dc01..62da327931f 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -171,8 +171,6 @@ module Gitlab when /\AMilestone:/ "#fee3ff" - when *@closed_statuses.map { |s| nice_status_name(s) } - "#cfcfcf" when "Status: New" "#428bca" when "Status: Accepted" @@ -199,6 +197,8 @@ module Gitlab "#8e44ad" when "Type: Other" "#7f8c8d" + when *@closed_statuses.map { |s| nice_status_name(s) } + "#cfcfcf" else "#e2e2e2" end @@ -227,7 +227,7 @@ module Gitlab s = s.gsub("`", "\\`") # Carriage returns make me sad - s = s.gsub("\r", "") + s = s.delete("\r") # Markdown ignores single newlines, but we need them as
. s = s.gsub("\n", " \n") diff --git a/lib/gitlab/google_code_import/project_creator.rb b/lib/gitlab/google_code_import/project_creator.rb index 1cb7d16aeb3..87821c23460 100644 --- a/lib/gitlab/google_code_import/project_creator.rb +++ b/lib/gitlab/google_code_import/project_creator.rb @@ -11,7 +11,8 @@ module Gitlab end def execute - project = ::Projects::CreateService.new(current_user, + project = ::Projects::CreateService.new( + current_user, name: repo.name, path: repo.name, description: repo.summary, diff --git a/lib/rouge/formatters/html_gitlab.rb b/lib/rouge/formatters/html_gitlab.rb index 6762ca47c32..8c309efc7b8 100644 --- a/lib/rouge/formatters/html_gitlab.rb +++ b/lib/rouge/formatters/html_gitlab.rb @@ -39,7 +39,7 @@ module Rouge lineanchorsid: 'L', anchorlinenos: false, inline_theme: nil - ) + ) @nowrap = nowrap @cssclass = cssclass @linenos = linenos diff --git a/spec/factories.rb b/spec/factories.rb index 4bf93adabe2..d6b4efa9a03 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -43,7 +43,8 @@ FactoryGirl.define do end after(:create) do |user, evaluator| - user.identities << create(:identity, + user.identities << create( + :identity, provider: evaluator.provider, extern_uid: evaluator.extern_uid ) diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group_access_spec.rb index 4b78e3a61f0..65f8073c693 100644 --- a/spec/features/security/group_access_spec.rb +++ b/spec/features/security/group_access_spec.rb @@ -16,11 +16,11 @@ describe 'Group access', feature: true do end end - def group_member(access_level, group = group) + def group_member(access_level, grp = group()) level = Object.const_get("Gitlab::Access::#{access_level.upcase}") create(:user).tap do |user| - group.add_user(user, level) + grp.add_user(user, level) end end diff --git a/spec/helpers/groups_helper.rb b/spec/helpers/groups_helper.rb index 5d174460681..4ea90a80a92 100644 --- a/spec/helpers/groups_helper.rb +++ b/spec/helpers/groups_helper.rb @@ -9,7 +9,7 @@ describe GroupsHelper do group.avatar = File.open(avatar_file_path) group.save! expect(group_icon(group.path).to_s). - to match("/uploads/group/avatar/#{ group.id }/banana_sample.gif") + to match("/uploads/group/avatar/#{group.id}/banana_sample.gif") end it 'should give default avatar_icon when no avatar is present' do diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index ac61c8fb525..b193e16e7f8 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -37,14 +37,14 @@ describe Ci::Commit, models: true do it 'returns ordered list of commits' do commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project - commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, project: project + commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project expect(project.ci_commits.ordered).to eq([commit2, commit1]) end it 'returns commits ordered by committed_at and id, with nulls last' do commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project commit2 = FactoryGirl.create :ci_commit, committed_at: nil, project: project - commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hour.ago, project: project + commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project commit4 = FactoryGirl.create :ci_commit, committed_at: nil, project: project expect(project.ci_commits.ordered).to eq([commit2, commit4, commit3, commit1]) end diff --git a/spec/models/key_spec.rb b/spec/models/key_spec.rb index d7fe01976d8..c962b83644a 100644 --- a/spec/models/key_spec.rb +++ b/spec/models/key_spec.rb @@ -81,7 +81,7 @@ describe Key, models: true do it 'rejects the multiple line key' do key = build(:key) - key.key.gsub!(' ', "\n") + key.key.tr!(' ', "\n") expect(key).not_to be_valid end end diff --git a/spec/models/project_services/hipchat_service_spec.rb b/spec/models/project_services/hipchat_service_spec.rb index a5662b08bda..91dd92b7c67 100644 --- a/spec/models/project_services/hipchat_service_spec.rb +++ b/spec/models/project_services/hipchat_service_spec.rb @@ -57,23 +57,21 @@ describe HipchatService, models: true do it 'should use v1 if version is provided' do allow(hipchat).to receive(:api_version).and_return('v1') - expect(HipChat::Client).to receive(:new). - with(token, - api_version: 'v1', - server_url: server_url). - and_return( - double(:hipchat_service).as_null_object) + expect(HipChat::Client).to receive(:new).with( + token, + api_version: 'v1', + server_url: server_url + ).and_return(double(:hipchat_service).as_null_object) hipchat.execute(push_sample_data) end it 'should use v2 as the version when nothing is provided' do allow(hipchat).to receive(:api_version).and_return('') - expect(HipChat::Client).to receive(:new). - with(token, - api_version: 'v2', - server_url: server_url). - and_return( - double(:hipchat_service).as_null_object) + expect(HipChat::Client).to receive(:new).with( + token, + api_version: 'v2', + server_url: server_url + ).and_return(double(:hipchat_service).as_null_object) hipchat.execute(push_sample_data) end diff --git a/spec/models/project_services/slack_service/note_message_spec.rb b/spec/models/project_services/slack_service/note_message_spec.rb index ebf8837570e..06006b9a4f5 100644 --- a/spec/models/project_services/slack_service/note_message_spec.rb +++ b/spec/models/project_services/slack_service/note_message_spec.rb @@ -89,10 +89,10 @@ describe SlackService::NoteMessage, models: true do it 'returns a message regarding notes on an issue' do message = SlackService::NoteMessage.new(@args) expect(message.pretext).to eq( - "Test User commented on " \ - " in : " \ - "*issue title*") - expected_attachments = [ + "Test User commented on " \ + " in : " \ + "*issue title*") + expected_attachments = [ { text: "comment on an issue", color: color, diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index daa9d1087bf..376266c0955 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -462,8 +462,8 @@ describe User, models: true do expect(User.search(user1.username.downcase).to_a).to eq([user1]) expect(User.search(user2.username.upcase).to_a).to eq([user2]) expect(User.search(user2.username.downcase).to_a).to eq([user2]) - expect(User.search(user1.username.downcase).to_a.count).to eq(2) - expect(User.search(user2.username.downcase).to_a.count).to eq(1) + expect(User.search(user1.username.downcase).to_a.size).to eq(2) + expect(User.search(user2.username.downcase).to_a.size).to eq(1) end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index a91fa735321..e194eb93cf4 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -6,7 +6,7 @@ describe API::API, api: true do let(:user) { create(:user) } let!(:project) {create(:project, creator_id: user.id, namespace: user.namespace) } let!(:merge_request) { create(:merge_request, :simple, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) } - let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.seconds) } + let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) } let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds) } let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") } let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") } diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index b180d2fec77..fed9ae1949b 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -29,7 +29,7 @@ describe API::API, api: true do if required_attributes.empty? expected_code = 200 else - attrs.delete(required_attributes.shuffle.first) + attrs.delete(required_attributes.sample) expected_code = 400 end diff --git a/spec/services/create_commit_builds_service_spec.rb b/spec/services/create_commit_builds_service_spec.rb index 798c480b81a..40ba6549e1f 100644 --- a/spec/services/create_commit_builds_service_spec.rb +++ b/spec/services/create_commit_builds_service_spec.rb @@ -17,7 +17,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: [{ message: "Message" }] - ) + ) end it { expect(commit).to be_kind_of(Ci::Commit) } @@ -34,7 +34,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: [{ message: "Message" }] - ) + ) expect(result).to be_persisted end @@ -47,7 +47,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: [{ message: "Message" }] - ) + ) expect(result).to be_persisted end end @@ -59,7 +59,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: [{ message: 'Message' }] - ) + ) expect(result).to be_persisted expect(result.builds.any?).to be_falsey expect(result.status).to eq('skipped') @@ -76,7 +76,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.any?).to be false expect(commit.status).to eq('failed') expect(commit.yaml_errors).to_not be_nil @@ -96,7 +96,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.any?).to be false expect(commit.status).to eq("skipped") end @@ -110,7 +110,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.first.name).to eq("staging") end @@ -123,7 +123,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.any?).to be false expect(commit.status).to eq("skipped") expect(commit.yaml_errors).to be_nil @@ -139,7 +139,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.count(:all)).to eq(2) commit = service.execute(project, user, @@ -147,7 +147,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.builds.count(:all)).to eq(2) end @@ -161,7 +161,7 @@ describe CreateCommitBuildsService, services: true do before: '00000000', after: '31das312', commits: commits - ) + ) expect(commit.status).to eq("failed") expect(commit.builds.any?).to be false diff --git a/spec/services/git_tag_push_service_spec.rb b/spec/services/git_tag_push_service_spec.rb index e2d15f1a83d..b982274c529 100644 --- a/spec/services/git_tag_push_service_spec.rb +++ b/spec/services/git_tag_push_service_spec.rb @@ -58,14 +58,14 @@ describe GitTagPushService, services: true do it { is_expected.to include(timestamp: @commit.date.xmlschema) } it do is_expected.to include( - url: [ - Gitlab.config.gitlab.url, - project.namespace.to_param, - project.to_param, - 'commit', - @commit.id - ].join('/') - ) + url: [ + Gitlab.config.gitlab.url, + project.namespace.to_param, + project.to_param, + 'commit', + @commit.id + ].join('/') + ) end context "with a author" do diff --git a/spec/services/update_snippet_service_spec.rb b/spec/services/update_snippet_service_spec.rb index 124bb76e678..48d114896d0 100644 --- a/spec/services/update_snippet_service_spec.rb +++ b/spec/services/update_snippet_service_spec.rb @@ -42,7 +42,7 @@ describe UpdateSnippetService, services: true do CreateSnippetService.new(project, user, opts).execute end - def update_snippet(project = nil, user, snippet, opts) + def update_snippet(project, user, snippet, opts) UpdateSnippetService.new(project, user, snippet, opts).execute end end diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb index aadf791bf3f..aa8258d6dad 100644 --- a/spec/support/repo_helpers.rb +++ b/spec/support/repo_helpers.rb @@ -45,12 +45,12 @@ eos def another_sample_commit OpenStruct.new( - id: "e56497bb5f03a90a51293fc6d516788730953899", - parent_id: '4cd80ccab63c82b4bad16faa5193fbd2aa06df40', - author_full_name: "Sytse Sijbrandij", - author_email: "sytse@gitlab.com", - files_changed_count: 1, - message: <