Commit 172cb70d authored by Robert Speicher's avatar Robert Speicher

Merge branch '35793_fix_predicate_names' into 'master'

Remove `is_` prefix from predicate method names

See merge request !13810
parents 8e606369 380dff62
......@@ -606,6 +606,18 @@ Style/YodaCondition:
Style/Proc:
Enabled: true
# Use `spam?` instead of `is_spam?`
# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
# NamePrefix: is_, has_, have_
# NamePrefixBlacklist: is_, has_, have_
# NameWhitelist: is_a?
Style/PredicateName:
Enabled: true
NamePrefixBlacklist: is_
Exclude:
- 'spec/**/*'
- 'features/**/*'
# Metrics #####################################################################
# A calculated magnitude based on number of assignments,
......
......@@ -237,14 +237,6 @@ Style/PercentLiteralDelimiters:
Style/PerlBackrefs:
Enabled: false
# Offense count: 105
# Configuration parameters: NamePrefix, NamePrefixBlacklist, NameWhitelist.
# NamePrefix: is_, has_, have_
# NamePrefixBlacklist: is_, has_, have_
# NameWhitelist: is_a?
Style/PredicateName:
Enabled: false
# Offense count: 58
# Cop supports --auto-correct.
# Configuration parameters: EnforcedStyle, SupportedStyles.
......
......@@ -35,13 +35,13 @@ class Groups::MilestonesController < Groups::ApplicationController
end
def edit
render_404 if @milestone.is_legacy_group_milestone?
render_404 if @milestone.legacy_group_milestone?
end
def update
# Keep this compatible with legacy group milestones where we have to update
# all projects milestones states at once.
if @milestone.is_legacy_group_milestone?
if @milestone.legacy_group_milestone?
update_params = milestone_params.select { |key| key == "state_event" }
milestones = @milestone.milestones
else
......@@ -67,7 +67,7 @@ class Groups::MilestonesController < Groups::ApplicationController
end
def milestone_path
if @milestone.is_legacy_group_milestone?
if @milestone.legacy_group_milestone?
group_milestone_path(group, @milestone.safe_title, title: @milestone.title)
else
group_milestone_path(group, @milestone.iid)
......
......@@ -202,7 +202,7 @@ class Projects::IssuesController < Projects::ApplicationController
task_status: @issue.task_status
}
if @issue.is_edited?
if @issue.edited?
response[:updated_at] = @issue.updated_at
response[:updated_by_name] = @issue.last_edited_by.name
response[:updated_by_path] = user_path(@issue.last_edited_by)
......
......@@ -178,7 +178,7 @@ module ApplicationHelper
end
def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', exclude_author: false)
return unless object.is_edited?
return unless object.edited?
content_tag :small, class: 'edited-text' do
output = content_tag(:span, 'Edited ')
......
......@@ -229,7 +229,7 @@ module IssuablesHelper
end
def updated_at_by(issuable)
return {} unless issuable.is_edited?
return {} unless issuable.edited?
{
updatedAt: issuable.updated_at.to_time.iso8601,
......
......@@ -164,7 +164,7 @@ module MilestonesHelper
def group_milestone_route(milestone, params = {})
params = nil if params.empty?
if milestone.is_legacy_group_milestone?
if milestone.legacy_group_milestone?
group_milestone_path(@group, milestone.safe_title, title: milestone.title, milestone: params)
else
group_milestone_path(@group, milestone.iid, milestone: params)
......
module MilestonesRoutingHelper
def milestone_path(milestone, *args)
if milestone.is_group_milestone?
if milestone.group_milestone?
group_milestone_path(milestone.group, milestone, *args)
elsif milestone.is_project_milestone?
elsif milestone.project_milestone?
project_milestone_path(milestone.project, milestone, *args)
end
end
def milestone_url(milestone, *args)
if milestone.is_group_milestone?
if milestone.group_milestone?
group_milestone_url(milestone.group, milestone, *args)
elsif milestone.is_project_milestone?
elsif milestone.project_milestone?
project_milestone_url(milestone.project, milestone, *args)
end
end
......
......@@ -142,7 +142,7 @@ module Ci
expire: RUNNER_QUEUE_EXPIRY_TIME, overwrite: false)
end
def is_runner_queue_value_latest?(value)
def runner_queue_value_latest?(value)
ensure_runner_queue_value == value if value.present?
end
......
module Editable
extend ActiveSupport::Concern
def is_edited?
def edited?
last_edited_at.present? && last_edited_at != created_at
end
......
......@@ -70,19 +70,19 @@ module Milestoneish
due_date && due_date.past?
end
def is_group_milestone?
def group_milestone?
false
end
def is_project_milestone?
def project_milestone?
false
end
def is_legacy_group_milestone?
def legacy_group_milestone?
false
end
def is_dashboard_milestone?
def dashboard_milestone?
false
end
......
......@@ -3,7 +3,7 @@ class DashboardMilestone < GlobalMilestone
{ authorized_only: true }
end
def is_dashboard_milestone?
def dashboard_milestone?
true
end
end
......@@ -49,7 +49,7 @@ class Deployment < ActiveRecord::Base
# created before then could have a `sha` referring to a commit that no
# longer exists in the repository, so just ignore those.
begin
project.repository.is_ancestor?(commit.id, sha)
project.repository.ancestor?(commit.id, sha)
rescue Rugged::OdbError
false
end
......
......@@ -17,7 +17,7 @@ class GroupMilestone < GlobalMilestone
{ group_id: group.id }
end
def is_legacy_group_milestone?
def legacy_group_milestone?
true
end
end
......@@ -163,7 +163,7 @@ class Milestone < ActiveRecord::Base
# Milestone.first.to_reference(same_namespace_project) # => "gitlab-ce%1"
#
def to_reference(from_project = nil, format: :iid, full: false)
return if is_group_milestone? && format != :name
return if group_milestone? && format != :name
format_reference = milestone_format_reference(format)
reference = "#{self.class.reference_prefix}#{format_reference}"
......@@ -207,11 +207,11 @@ class Milestone < ActiveRecord::Base
group || project
end
def is_group_milestone?
def group_milestone?
group_id.present?
end
def is_project_milestone?
def project_milestone?
project_id.present?
end
......
......@@ -152,14 +152,14 @@ module Network
end
def find_free_parent_space(range, space_base, space_step, space_default)
if is_overlap?(range, space_default)
if overlap?(range, space_default)
find_free_space(range, space_step, space_base, space_default)
else
space_default
end
end
def is_overlap?(range, overlap_space)
def overlap?(range, overlap_space)
range.each do |i|
if i != range.first &&
i != range.last &&
......
......@@ -101,9 +101,9 @@ class ChatNotificationService < Service
when "push", "tag_push"
ChatMessage::PushMessage.new(data)
when "issue"
ChatMessage::IssueMessage.new(data) unless is_update?(data)
ChatMessage::IssueMessage.new(data) unless update?(data)
when "merge_request"
ChatMessage::MergeMessage.new(data) unless is_update?(data)
ChatMessage::MergeMessage.new(data) unless update?(data)
when "note"
ChatMessage::NoteMessage.new(data)
when "pipeline"
......@@ -136,7 +136,7 @@ class ChatNotificationService < Service
project.web_url
end
def is_update?(data)
def update?(data)
data[:object_attributes][:action] == 'update'
end
......
......@@ -85,9 +85,9 @@ class HipchatService < Service
when "push", "tag_push"
create_push_message(data)
when "issue"
create_issue_message(data) unless is_update?(data)
create_issue_message(data) unless update?(data)
when "merge_request"
create_merge_request_message(data) unless is_update?(data)
create_merge_request_message(data) unless update?(data)
when "note"
create_note_message(data)
when "pipeline"
......@@ -282,7 +282,7 @@ class HipchatService < Service
"<a href=\"#{project_url}\">#{project_name}</a>"
end
def is_update?(data)
def update?(data)
data[:object_attributes][:action] == 'update'
end
......
......@@ -944,7 +944,7 @@ class Repository
if branch_commit
same_head = branch_commit.id == root_ref_commit.id
!same_head && is_ancestor?(branch_commit.id, root_ref_commit.id)
!same_head && ancestor?(branch_commit.id, root_ref_commit.id)
else
nil
end
......@@ -958,12 +958,12 @@ class Repository
nil
end
def is_ancestor?(ancestor_id, descendant_id)
def ancestor?(ancestor_id, descendant_id)
return false if ancestor_id.nil? || descendant_id.nil?
Gitlab::GitalyClient.migrate(:is_ancestor) do |is_enabled|
if is_enabled
raw_repository.is_ancestor?(ancestor_id, descendant_id)
raw_repository.ancestor?(ancestor_id, descendant_id)
else
rugged_is_ancestor?(ancestor_id, descendant_id)
end
......
......@@ -17,13 +17,13 @@ class ProjectPolicy < BasePolicy
desc "Project has public builds enabled"
condition(:public_builds, scope: :subject) { project.public_builds? }
# For guest access we use #is_team_member? so we can use
# For guest access we use #team_member? so we can use
# project.members, which gets cached in subject scope.
# This is safe because team_access_level is guaranteed
# by ProjectAuthorization's validation to be at minimum
# GUEST
desc "User has guest access"
condition(:guest) { is_team_member? }
condition(:guest) { team_member? }
desc "User has reporter access"
condition(:reporter) { team_access_level >= Gitlab::Access::REPORTER }
......@@ -293,7 +293,7 @@ class ProjectPolicy < BasePolicy
private
def is_team_member?
def team_member?
return false if @user.nil?
greedy_load_subject = false
......
......@@ -7,7 +7,7 @@ class AkismetService
@options = options
end
def is_spam?
def spam?
return false unless akismet_enabled?
params = {
......
......@@ -30,7 +30,7 @@ class GitPushService < BaseService
@project.repository.after_create_branch
# Re-find the pushed commits.
if is_default_branch?
if default_branch?
# Initial push to the default branch. Take the full history of that branch as "newly pushed".
process_default_branch
else
......@@ -50,7 +50,7 @@ class GitPushService < BaseService
# Update the bare repositories info/attributes file using the contents of the default branches
# .gitattributes file
update_gitattributes if is_default_branch?
update_gitattributes if default_branch?
end
execute_related_hooks
......@@ -66,7 +66,7 @@ class GitPushService < BaseService
end
def update_caches
if is_default_branch?
if default_branch?
if push_to_new_branch?
# If this is the initial push into the default branch, the file type caches
# will already be reset as a result of `Project#change_head`.
......@@ -108,7 +108,7 @@ class GitPushService < BaseService
# Schedules processing of commit messages.
def process_commit_messages
default = is_default_branch?
default = default_branch?
@push_commits.last(PROCESS_COMMIT_LIMIT).each do |commit|
if commit.matches_cross_reference_regex?
......@@ -202,7 +202,7 @@ class GitPushService < BaseService
Gitlab::Git.branch_ref?(params[:ref])
end
def is_default_branch?
def default_branch?
Gitlab::Git.branch_ref?(params[:ref]) &&
(Gitlab::Git.ref_name(params[:ref]) == project.default_branch || project.default_branch.nil?)
end
......
module Milestones
class CloseService < Milestones::BaseService
def execute(milestone)
if milestone.close && milestone.is_project_milestone?
if milestone.close && milestone.project_milestone?
event_service.close_milestone(milestone, current_user)
end
......
......@@ -3,7 +3,7 @@ module Milestones
def execute
milestone = parent.milestones.new(params)
if milestone.save && milestone.is_project_milestone?
if milestone.save && milestone.project_milestone?
event_service.open_milestone(milestone, current_user)
end
......
module Milestones
class DestroyService < Milestones::BaseService
def execute(milestone)
return unless milestone.is_project_milestone?
return unless milestone.project_milestone?
Milestone.transaction do
update_params = { milestone: nil }
......
module Milestones
class ReopenService < Milestones::BaseService
def execute(milestone)
if milestone.activate && milestone.is_project_milestone?
if milestone.activate && milestone.project_milestone?
event_service.reopen_milestone(milestone, current_user)
end
......
......@@ -45,7 +45,7 @@ class SpamService
def check(api)
return false unless request && check_for_spam?
return false unless akismet.is_spam?
return false unless akismet.spam?
create_spam_log(api)
true
......
......@@ -142,7 +142,7 @@ module SystemNoteService
#
# Returns the created Note object
def change_milestone(noteable, project, author, milestone)
format = milestone&.is_group_milestone? ? :name : :iid
format = milestone&.group_milestone? ? :name : :iid
body = milestone.nil? ? 'removed milestone' : "changed milestone to #{milestone.to_reference(project, format: format)}"
create_note(NoteSummary.new(noteable, project, author, body, action: 'milestone'))
......
= render "header_title"
= render 'shared/milestones/top', milestone: @milestone, group: @group
= render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true if @milestone.is_legacy_group_milestone?
= render 'shared/milestones/tabs', milestone: @milestone, show_project_name: true if @milestone.legacy_group_milestone?
= render 'shared/milestones/sidebar', milestone: @milestone, affix_offset: 102
......@@ -5,7 +5,7 @@
.row
.col-sm-6
%strong= link_to truncate(milestone.title, length: 100), milestone_path
- if milestone.is_group_milestone?
- if milestone.group_milestone?
%span - Group Milestone
- else
%span - Project Milestone
......@@ -18,10 +18,10 @@
&middot;
= link_to pluralize(milestone.merge_requests.size, 'Merge Request'), merge_requests_path
.col-sm-6= milestone_progress_bar(milestone)
- if milestone.is_a?(GlobalMilestone) || milestone.is_group_milestone?
- if milestone.is_a?(GlobalMilestone) || milestone.group_milestone?
.row
.col-sm-6
- if milestone.is_legacy_group_milestone?
- if milestone.legacy_group_milestone?
.expiration= render('shared/milestone_expired', milestone: milestone)
.projects
- milestone.milestones.each do |milestone|
......@@ -31,7 +31,7 @@
- if @group
.col-sm-6.milestone-actions
- if can?(current_user, :admin_milestones, @group)
- if milestone.is_group_milestone?
- if milestone.group_milestone?
= link_to edit_group_milestone_path(@group, milestone), class: "btn btn-xs btn-grouped" do
Edit
\
......
......@@ -22,7 +22,7 @@
- if group
.pull-right
- if can?(current_user, :admin_milestones, group)
- if milestone.is_group_milestone?
- if milestone.group_milestone?
= link_to edit_group_milestone_path(group, milestone), class: "btn btn btn-grouped" do
Edit
- if milestone.active?
......@@ -33,7 +33,7 @@
.detail-page-description.milestone-detail
%h2.title
= markdown_field(milestone, :title)
- if @milestone.is_group_milestone? && @milestone.description.present?
- if @milestone.group_milestone? && @milestone.description.present?
%div
.description
.wiki
......@@ -44,7 +44,7 @@
- close_msg = group ? 'You may close the milestone now.' : 'Navigate to the project to close the milestone.'
%span All issues for this milestone are closed. #{close_msg}
- if @milestone.is_legacy_group_milestone? || @milestone.is_dashboard_milestone?
- if @milestone.legacy_group_milestone? || @milestone.dashboard_milestone?
.table-holder
%table.table
%thead
......@@ -67,7 +67,7 @@
Open
%td
= ms.expires_at
- elsif @milestone.is_group_milestone?
- elsif @milestone.group_milestone?
%br
View
= link_to 'Issues', issues_group_path(@group, milestone_title: milestone.title)
......
---
title: Remove `is_` prefix from predicate method names
merge_request: 13810
author: Maxim Rydkin
type: other
......@@ -78,7 +78,7 @@ module API
no_content! unless current_runner.active?
update_runner_info
if current_runner.is_runner_queue_value_latest?(params[:last_update])
if current_runner.runner_queue_value_latest?(params[:last_update])
header 'X-GitLab-Last-Update', params[:last_update]
Gitlab::Metrics.add_event(:build_not_found_cached)
return no_content!
......
......@@ -12,7 +12,7 @@ module Gitlab
!project
.repository
.gitaly_commit_client
.is_ancestor(oldrev, newrev)
.ancestor?(oldrev, newrev)
else
Gitlab::Git::RevList.new(
path_to_repo: project.repository.path_to_repo,
......
......@@ -439,8 +439,8 @@ module Gitlab
end
# Returns true is +from+ is direct ancestor to +to+, otherwise false
def is_ancestor?(from, to)
gitaly_commit_client.is_ancestor(from, to)
def ancestor?(from, to)
gitaly_commit_client.ancestor?(from, to)
end
# Return an array of Diff objects that represent the diff
......
......@@ -22,7 +22,7 @@ module Gitlab
end
end
def is_ancestor(ancestor_id, child_id)
def ancestor?(ancestor_id, child_id)
request = Gitaly::CommitIsAncestorRequest.new(
repository: @gitaly_repo,
ancestor_id: ancestor_id,
......
......@@ -10,7 +10,7 @@ module Gitlab
'db_ping'
end
def is_successful?(result)
def successful?(result)
result == '1'
end
......
......@@ -15,7 +15,7 @@ module Gitlab
'redis_cache_ping'
end
def is_successful?(result)
def successful?(result)
result == 'PONG'
end
......
......@@ -15,7 +15,7 @@ module Gitlab
'redis_queues_ping'
end
def is_successful?(result)
def successful?(result)
result == 'PONG'
end
......
......@@ -11,7 +11,7 @@ module Gitlab
'redis_ping'
end
def is_successful?(result)
def successful?(result)
result == 'PONG'
end
......
......@@ -15,7 +15,7 @@ module Gitlab
'redis_shared_state_ping'
end
def is_successful?(result)
def successful?(result)
result == 'PONG'
end
......
......@@ -5,7 +5,7 @@ module Gitlab
def readiness
check_result = check
if is_successful?(check_result)
if successful?(check_result)
HealthChecks::Result.new(true)
elsif check_result.is_a?(Timeout::Error)
HealthChecks::Result.new(false, "#{human_name} check timed out")
......@@ -16,10 +16,10 @@ module Gitlab
def metrics
result, elapsed = with_timing(&method(:check))
Rails.logger.error("#{human_name} check returned unexpected result #{result}") unless is_successful?(result)
Rails.logger.error("#{human_name} check returned unexpected result #{result}") unless successful?(result)
[
metric("#{metric_prefix}_timeout", result.is_a?(Timeout::Error) ? 1 : 0),
metric("#{metric_prefix}_success", is_successful?(result) ? 1 : 0),
metric("#{metric_prefix}_success", successful?(result) ? 1 : 0),
metric("#{metric_prefix}_latency_seconds", elapsed)
]
end
......@@ -30,7 +30,7 @@ module Gitlab
raise NotImplementedError
end
def is_successful?(result)
def successful?(result)
raise NotImplementedError
end
......
......@@ -42,13 +42,13 @@ module Gitlab
lookup = series.each_slice(MAX_QUERY_ITEMS).flat_map do |batched_series|
client_series(*batched_series, start: timeframe_start, stop: timeframe_end)
.select(&method(:has_matching_label))
.select(&method(:has_matching_label?))
.map { |series_info| [series_info['__name__'], true] }
end
lookup.to_h
end
def has_matching_label(series_info)
def has_matching_label?(series_info)
series_info.key?('environment')
end
......
......@@ -20,7 +20,7 @@ module SystemCheck
# Returns true if all subcommands were successful (according to their exit code)
# Returns false if any or all subcommands failed.
def repair!
return false unless is_gitlab_user?
return false unless gitlab_user?
command_success = OPTIONS.map do |name, value|
system(*%W(#{Gitlab.config.git.bin_path} config --global #{name} #{value}))
......
......@@ -73,7 +73,7 @@ module SystemCheck
self.class.instance_methods(false).include?(:skip?)
end
def is_multi_check?
def multi_check?
self.class.instance_methods(false).include?(:multi_check)
end
......
......@@ -53,7 +53,7 @@ module SystemCheck
end
# When implements a multi check, we don't control the output
if check.is_multi_check?
if check.multi_check?
check.multi_check
return
end
......
......@@ -104,7 +104,7 @@ module Gitlab
Gitlab.config.gitlab.user
end
def is_gitlab_user?
def gitlab_user?
return @is_gitlab_user unless @is_gitlab_user.nil?
current_user = run_command(%w(whoami)).chomp
......@@ -114,7 +114,7 @@ module Gitlab
def warn_user_is_not_gitlab
return if @warned_user_not_gitlab
unless is_gitlab_user?
unless gitlab_user?
current_user = run_command(%w(whoami)).chomp
puts " Warning ".color(:black).background(:yellow)
......
......@@ -268,7 +268,7 @@ describe Projects::IssuesController do
context 'when an issue is not identified as spam' do
before do
allow_any_instance_of(described_class).to receive(:verify_recaptcha).and_return(false)
allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(false)
allow_any_instance_of(AkismetService).to receive(:spam?).and_return(false)
end
it 'normally updates the issue' do
......@@ -278,7 +278,7 @@ describe Projects::IssuesController do