diff options
Diffstat (limited to 'lib/gitlab/danger')
| -rw-r--r-- | lib/gitlab/danger/base_linter.rb | 95 | ||||
| -rw-r--r-- | lib/gitlab/danger/changelog.rb | 1 | ||||
| -rw-r--r-- | lib/gitlab/danger/commit_linter.rb | 118 | ||||
| -rw-r--r-- | lib/gitlab/danger/helper.rb | 6 | ||||
| -rw-r--r-- | lib/gitlab/danger/merge_request_linter.rb | 30 | ||||
| -rw-r--r-- | lib/gitlab/danger/roulette.rb | 2 |
6 files changed, 154 insertions, 98 deletions
diff --git a/lib/gitlab/danger/base_linter.rb b/lib/gitlab/danger/base_linter.rb new file mode 100644 index 00000000000..df2e9e745aa --- /dev/null +++ b/lib/gitlab/danger/base_linter.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +module Gitlab + module Danger + class BaseLinter + MIN_SUBJECT_WORDS_COUNT = 3 + MAX_LINE_LENGTH = 72 + WIP_PREFIX = 'WIP: ' + + attr_reader :commit, :problems + + def self.problems_mapping + { + subject_too_short: "The %s must contain at least #{MIN_SUBJECT_WORDS_COUNT} words", + subject_too_long: "The %s may not be longer than #{MAX_LINE_LENGTH} characters", + subject_starts_with_lowercase: "The %s must start with a capital letter", + subject_ends_with_a_period: "The %s must not end with a period" + } + end + + def self.subject_description + 'commit subject' + end + + def initialize(commit) + @commit = commit + @problems = {} + end + + def failed? + problems.any? + end + + def add_problem(problem_key, *args) + @problems[problem_key] = sprintf(self.class.problems_mapping[problem_key], *args) + end + + def lint_subject + if subject_too_short? + add_problem(:subject_too_short, self.class.subject_description) + end + + if subject_too_long? + add_problem(:subject_too_long, self.class.subject_description) + end + + if subject_starts_with_lowercase? + add_problem(:subject_starts_with_lowercase, self.class.subject_description) + end + + if subject_ends_with_a_period? + add_problem(:subject_ends_with_a_period, self.class.subject_description) + end + + self + end + + private + + def subject + message_parts[0].delete_prefix(WIP_PREFIX) + end + + def subject_too_short? + subject.split(' ').length < MIN_SUBJECT_WORDS_COUNT + end + + def subject_too_long? + line_too_long?(subject) + end + + def line_too_long?(line) + line.length > MAX_LINE_LENGTH + end + + def subject_starts_with_lowercase? + return false if ('A'..'Z').cover?(subject[0]) + + first_char = subject.sub(/\A(\[.+\]|\w+:)\s/, '')[0] + first_char_downcased = first_char.downcase + return true unless ('a'..'z').cover?(first_char_downcased) + + first_char.downcase == first_char + end + + def subject_ends_with_a_period? + subject.end_with?('.') + end + + def message_parts + @message_parts ||= commit.message.split("\n", 3) + end + end + end +end diff --git a/lib/gitlab/danger/changelog.rb b/lib/gitlab/danger/changelog.rb index 607ca1200a0..92af6849b2f 100644 --- a/lib/gitlab/danger/changelog.rb +++ b/lib/gitlab/danger/changelog.rb @@ -39,6 +39,7 @@ module Gitlab def required? git.added_files.any? { |path| path =~ %r{\Adb/(migrate|post_migrate)/} } end + alias_method :db_changes?, :required? def optional? categories_need_changelog? && without_no_changelog_label? diff --git a/lib/gitlab/danger/commit_linter.rb b/lib/gitlab/danger/commit_linter.rb index 2e469359bdc..7e2e0fb0acb 100644 --- a/lib/gitlab/danger/commit_linter.rb +++ b/lib/gitlab/danger/commit_linter.rb @@ -1,40 +1,37 @@ # frozen_string_literal: true +require_relative 'base_linter' + emoji_checker_path = File.expand_path('emoji_checker', __dir__) defined?(Rails) ? require_dependency(emoji_checker_path) : require_relative(emoji_checker_path) module Gitlab module Danger - class CommitLinter - MIN_SUBJECT_WORDS_COUNT = 3 - MAX_LINE_LENGTH = 72 + class CommitLinter < BaseLinter MAX_CHANGED_FILES_IN_COMMIT = 3 MAX_CHANGED_LINES_IN_COMMIT = 30 SHORT_REFERENCE_REGEX = %r{([\w\-\/]+)?(?<!`)(#|!|&|%)\d+(?<!`)}.freeze - DEFAULT_SUBJECT_DESCRIPTION = 'commit subject' - WIP_PREFIX = 'WIP: ' - PROBLEMS = { - subject_too_short: "The %s must contain at least #{MIN_SUBJECT_WORDS_COUNT} words", - subject_too_long: "The %s may not be longer than #{MAX_LINE_LENGTH} characters", - subject_starts_with_lowercase: "The %s must start with a capital letter", - subject_ends_with_a_period: "The %s must not end with a period", - separator_missing: "The commit subject and body must be separated by a blank line", - details_too_many_changes: "Commits that change #{MAX_CHANGED_LINES_IN_COMMIT} or more lines across " \ + + def self.problems_mapping + super.merge( + { + separator_missing: "The commit subject and body must be separated by a blank line", + details_too_many_changes: "Commits that change #{MAX_CHANGED_LINES_IN_COMMIT} or more lines across " \ "at least #{MAX_CHANGED_FILES_IN_COMMIT} files must describe these changes in the commit body", - details_line_too_long: "The commit body should not contain more than #{MAX_LINE_LENGTH} characters per line", - message_contains_text_emoji: "Avoid the use of Markdown Emoji such as `:+1:`. These add limited value " \ + details_line_too_long: "The commit body should not contain more than #{MAX_LINE_LENGTH} characters per line", + message_contains_text_emoji: "Avoid the use of Markdown Emoji such as `:+1:`. These add limited value " \ "to the commit message, and are displayed as plain text outside of GitLab", - message_contains_unicode_emoji: "Avoid the use of Unicode Emoji. These add no value to the commit " \ + message_contains_unicode_emoji: "Avoid the use of Unicode Emoji. These add no value to the commit " \ "message, and may not be displayed properly everywhere", - message_contains_short_reference: "Use full URLs instead of short references (`gitlab-org/gitlab#123` or " \ + message_contains_short_reference: "Use full URLs instead of short references (`gitlab-org/gitlab#123` or " \ "`!123`), as short references are displayed as plain text outside of GitLab" - }.freeze - - attr_reader :commit, :problems + } + ) + end def initialize(commit) - @commit = commit - @problems = {} + super + @linted = false end @@ -58,19 +55,11 @@ module Gitlab !details.nil? && !details.empty? end - def failed? - problems.any? - end - - def add_problem(problem_key, *args) - @problems[problem_key] = sprintf(PROBLEMS[problem_key], *args) - end - - def lint(subject_description = "commit subject") + def lint return self if @linted @linted = true - lint_subject(subject_description) + lint_subject lint_separator lint_details lint_message @@ -78,26 +67,6 @@ module Gitlab self end - def lint_subject(subject_description) - if subject_too_short? - add_problem(:subject_too_short, subject_description) - end - - if subject_too_long? - add_problem(:subject_too_long, subject_description) - end - - if subject_starts_with_lowercase? - add_problem(:subject_starts_with_lowercase, subject_description) - end - - if subject_ends_with_a_period? - add_problem(:subject_ends_with_a_period, subject_description) - end - - self - end - private def lint_separator @@ -114,15 +83,11 @@ module Gitlab end details&.each_line do |line| - line = line.strip - - next unless line_too_long?(line) - - url_size = line.scan(%r((https?://\S+))).sum { |(url)| url.length } + line_without_urls = line.strip.gsub(%r{https?://\S+}, '') # If the line includes a URL, we'll allow it to exceed MAX_LINE_LENGTH characters, but # only if the line _without_ the URL does not exceed this limit. - next unless line_too_long?(line.length - url_size) + next unless line_too_long?(line_without_urls) add_problem(:details_line_too_long) break @@ -159,10 +124,6 @@ module Gitlab files_changed > MAX_CHANGED_FILES_IN_COMMIT && lines_changed > MAX_CHANGED_LINES_IN_COMMIT end - def subject - message_parts[0].delete_prefix(WIP_PREFIX) - end - def separator message_parts[1] end @@ -171,37 +132,6 @@ module Gitlab message_parts[2]&.gsub(/^Signed-off-by.*$/, '') end - def line_too_long?(line) - case line - when String - line.length > MAX_LINE_LENGTH - when Integer - line > MAX_LINE_LENGTH - else - raise ArgumentError, "The line argument (#{line}) should be a String or an Integer! #{line.class} given." - end - end - - def subject_too_short? - subject.split(' ').length < MIN_SUBJECT_WORDS_COUNT - end - - def subject_too_long? - line_too_long?(subject) - end - - def subject_starts_with_lowercase? - first_char = subject.sub(/\A(\[.+\]|\w+:)\s/, '')[0] - first_char_downcased = first_char.downcase - return true unless ('a'..'z').cover?(first_char_downcased) - - first_char.downcase == first_char - end - - def subject_ends_with_a_period? - subject.end_with?('.') - end - def message_contains_text_emoji? emoji_checker.includes_text_emoji?(commit.message) end @@ -217,10 +147,6 @@ module Gitlab def emoji_checker @emoji_checker ||= Gitlab::Danger::EmojiChecker.new end - - def message_parts - @message_parts ||= commit.message.split("\n", 3) - end end end end diff --git a/lib/gitlab/danger/helper.rb b/lib/gitlab/danger/helper.rb index 89f21e8bd23..d22f28ff7f2 100644 --- a/lib/gitlab/danger/helper.rb +++ b/lib/gitlab/danger/helper.rb @@ -64,7 +64,7 @@ module Gitlab # - respond_to?(:gitlab) # - respond_to?(:gitlab, true) gitlab - rescue NoMethodError + rescue NameError nil end @@ -268,6 +268,10 @@ module Gitlab def has_database_scoped_labels?(current_mr_labels) current_mr_labels.any? { |label| label.start_with?('database::') } end + + def has_ci_changes? + changed_files(%r{\A(\.gitlab-ci\.yml|\.gitlab/ci/)}).any? + end end end end diff --git a/lib/gitlab/danger/merge_request_linter.rb b/lib/gitlab/danger/merge_request_linter.rb new file mode 100644 index 00000000000..d401d332aa7 --- /dev/null +++ b/lib/gitlab/danger/merge_request_linter.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require_relative 'base_linter' + +module Gitlab + module Danger + class MergeRequestLinter < BaseLinter + alias_method :lint, :lint_subject + + def self.subject_description + 'merge request title' + end + + def self.mr_run_options_regex + [ + 'RUN AS-IF-FOSS', + 'UPDATE CACHE', + 'RUN ALL RSPEC', + 'SKIP RSPEC FAIL-FAST' + ].join('|') + end + + private + + def subject + super.gsub(/\[?(#{self.class.mr_run_options_regex})\]?/, '').strip + end + end + end +end diff --git a/lib/gitlab/danger/roulette.rb b/lib/gitlab/danger/roulette.rb index 23f877b4e0f..328083f7002 100644 --- a/lib/gitlab/danger/roulette.rb +++ b/lib/gitlab/danger/roulette.rb @@ -24,7 +24,7 @@ module Gitlab # # @return [Array<Spin>] def spin(project, categories, timezone_experiment: false) - spins = categories.map do |category| + spins = categories.sort.map do |category| including_timezone = INCLUDE_TIMEZONE_FOR_CATEGORY.fetch(category, timezone_experiment) spin_for_category(project, category, timezone_experiment: including_timezone) |