Skip to content

Commit

Permalink
[Chore] Add danger checks
Browse files Browse the repository at this point in the history
Problem: it would be helpful to have checks for common rules on styling
commits and stuff, to avoid checking those manually every time.

Solution: add Danger checks, mostly the same we had in Morley.
  • Loading branch information
Martoon-00 committed Dec 20, 2022
1 parent 50e4e3b commit d259a3c
Show file tree
Hide file tree
Showing 8 changed files with 423 additions and 0 deletions.
33 changes: 33 additions & 0 deletions .github/workflows/danger.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# SPDX-FileCopyrightText: 2022 Serokell <https://serokell.io/>
#
# SPDX-License-Identifier: MPL-2.0

name: Danger

on: [pull_request]

jobs:
run-danger-checks:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: ruby/setup-ruby@v1
with:
ruby-version: '2.7'
bundler-cache: true
- uses: MeilCli/danger-action@v5
name: Instant checks
with:
install_path: 'vendor/bundle'
danger_file: './danger/instant-checks.rb'
danger_id: 'instant-checks'
env:
DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_BOT_TOKEN }}
- uses: MeilCli/danger-action@v5
name: Premerge checks
with:
install_path: 'vendor/bundle'
danger_file: './danger/premerge-checks.rb'
danger_id: 'premerge-checks'
env:
DANGER_GITHUB_API_TOKEN: ${{ secrets.DANGER_BOT_TOKEN }}
33 changes: 33 additions & 0 deletions danger/branch-name.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

require_relative 'helpers'

def check_branch_name
# Proper branch name
if branch_match = githost.branch_for_head.match(/([^\/]+)\/([^\-]+)-(.+)/)
nick, issue_id, desc = branch_match.captures

# We've decided not to put any restrictions on nickname for now

unless /^(#\d+|chore)$/.match?(issue_id)
warn(
"Bad issue ID in branch name.\n"\
"Valid format for issue IDs: `#123` or `chore`."
)
end

weird_chars = desc.scan(/[^a-zA-Z\-\d]/)
unless weird_chars.empty?
warn(
"Please, only use letters, digits and dashes in the branch name.
Found: #{weird_chars}"
)
end
elsif
warn(
"Please use `<nickname>/<issue-id>-<brief-description>`` format for branch names.`\n"\
"Example: `lazyman/#123-my-commit`"
)
end
end
93 changes: 93 additions & 0 deletions danger/commit-style.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

require_relative 'helpers'

def wrap_workarounds(fun)
return lambda { |msg|
method(fun).call(msg + "\nSee also \\[Note\\].")
markdown(
"\\[Note\\]: Skip this check by adding `wip`, `tmp` or `[temporary]` to the commit subject. "\
"Fixup commits (marked with `fixup!` or `squash!`) are also exempt from this check.")
}
end

def check_commit_style (mywarn = wrap_workarounds(:warn), myfail = wrap_workarounds(:fail))
# Proper commit style
# Note: we do not use commit_lint plugin because it triggers on fixup commits
git.commits.each { |commit|
if commit.fixup? || commit.wip?
next
end

subject = commit.subject
subject_payload = subject.sub(issue_tags_pattern, "")
subject_ticked = commit.subject_ticked

unless has_valid_issue_tags(subject)
# If any of these substrings is included into commit message,
# we are fine with issue tag absence.
exclusions = [
# In lower-case
"changelog"
]
if exclusions.none? { |exc| subject.downcase.include?(exc) }
mywarn.call("In #{commit.sha} message lacks issue id: #{subject_ticked}.")
end
end

if subject_payload.start_with?(" ")
mywarn.call("Extra space in commit #{commit.sha} subject after the issue tags: #{subject_ticked}.")
elsif !subject_payload.start_with?(/[A-Z]/)
mywarn.call("In #{commit.sha} subject does not begin with an uppercase letter: #{subject_ticked}.")
end

if subject[-1..-1] == '.'
mywarn.call("In #{commit.sha} message ends with a dot: #{subject_ticked} :fire_engine:")
end

if subject.length > 90
myfail.call("Nooo, such long commit message names do not work (#{commit.sha}).")
elsif subject.length > 72
mywarn.call("In commit #{commit.sha} message is too long (#{subject.length} chars), "\
"please keep its length within 72 characters.")
end

if commit.message_body.empty?
# If any of these substrings is included into commit message,
# we are fine with commit description absence.
exclusions = [
# In lower-case
"changelog"
]
unless commit.chore? || exclusions.any? { |exc| subject.downcase.include?(exc) }
myfail.call(
"Commit #{commit.sha} lacks description :unamused:\n"\
"Commits marked as `[Chore]` are exempt from this check."
)
end
else
# Checks on description

if !commit.blank_line_after_subject?
mywarn.call("In #{commit.sha} blank line is missing after the commit's subject.")
end

if !commit.chore?
description_patterns = [
/^Problem:[ \n].*^Solution:[ \n]/m,
/And yes, I don't care about templates/
]
unless description_patterns.any? { |pattern| pattern.match?(commit.description) }
mywarn.call(
"Description of #{commit.sha} does not follow the template.\n"\
"Try `Problem:`/`Solution:` structure.\n"\
"If you really have to, you can add `And yes, I don't care about templates` "\
"to the commit message body."
)
end
end
end

}
end
105 changes: 105 additions & 0 deletions danger/helpers.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

### Some convenient extensions and helpers ###

class Danger::Dangerfile
# Unified `github`/`gitlab` variable.
def githost
if defined?(github)
githost = github
elsif defined?(gitlab)
githost = gitlab
else
error "Failed to figure out which service are we running from."
end
end

# The original PR title (excludes "Draft" tags).
def pr_title_payload
githost.mr_title
.sub(/^(Draft|WIP): /, "")
.sub(/^\[Draft\] /, "")
end
alias_method :mr_title_payload, :pr_title_payload
end

class Git::Diff::DiffFile
# When a file is renamed (e.g. with `git mv`) 'path' will return the old
# path, this is true even if the file was modified a little.
# However we'd probably like to access the destination path instead, so
# this parses the new path from the 'file.patch'.
def destination_path
rename_match = /(?<=(\nrename to ))(\S)*/.match(self.patch)
if rename_match.nil?
self.path
else
rename_match.to_s
end
end
end

# Add some helpers to Commit class.
class Git::Object::Commit
# Commit subject (unlike the 'message' field which includes description).
def subject
self.message.lines.first.rstrip
end
alias_method :message_subject, :subject

def subject_ticked
"`" + self.subject.gsub("`", "'") + "`"
end

# Commit description.
# If absent, set to empty string.
def description
self.message.lines.drop(1).drop_while{ |s| s == "\n" }.join
end
alias_method :message_body, :description

# Whether there is a blank line between commit subject and body.
def blank_line_after_subject?
self.message.lines[1] == "\n"
end

# Whether this commit is fixup commit.
def fixup?
return /\bfixup!|\bsquash!/.match?(subject)
end

# Whether this commit is a temporary commit.
def wip?
return /\bwip\b|\btmp\b|\[temporary\]/i.match?(subject)
end

# Whether this commit is a minor chore commit.
# Such commits usually have an obvious purpose and are not related to the
# business logic.
def chore?
return subject.include?("[Chore]")
end

end

module Danger::Helpers::CommentsHelper
# By default, every comment for a particular source code also includes
# the name of the referred file.
#
# We don't need this feature.
# The source code welcomes us to override the respective method, and this is
# exactly what we do.
def markdown_link_to_message(_, _)
""
end
end

# Example: `[Chore][#123] My commit`
def issue_tags_pattern
/^(\[(#\d+|Chore)\])+ (?=\w)/
end

# Whether a string starts with an appropriate ticket tag.
def has_valid_issue_tags(name)
return name.start_with?(issue_tags_pattern)
end
34 changes: 34 additions & 0 deletions danger/instant-checks.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

# Checks that, when hit, should be fixed as soon as possible.

require_relative 'helpers'
require_relative 'trailing-whitespaces'
require_relative 'commit-style'
require_relative 'branch-name'
require_relative 'licenses'

check_trailing_whitespaces()

# Clean commits history
if git.commits.any? { |c| c.subject =~ /^Merge branch/ }
fail 'Please, no merge commits. Rebase for the win.'
end

check_commit_style()

# Proper MR content
mr_title_payload = githost.mr_title_payload

unless has_valid_issue_tags(mr_title_payload)
warn(
"Inappropriate title for PR.\n"\
"Should start from issue ID (e.g. `[#123]`) or `[Chore]` tag.\n"\
"Note: please use `[Chore]` also for tickets tracked internally on YouTrack."
)
end

check_branch_name()

check_licenses()
27 changes: 27 additions & 0 deletions danger/licenses.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

require_relative 'helpers'

def check_licenses
# Licenses
# Check that the REUSE license header contains the current year.
cur_year = Time.new.year
# Only go over new files; see https://gitlab.com/morley-framework/morley/-/merge_requests/1091
# for the discussion and rationale for this.
git.added_files.each do |file|
File.foreach(file).with_index(1).find do |line, line_num|
if year_match = line.match(/(^.*SPDX-FileCopyrightText:)\s+(\w+-)?(\w+)\s+(.*)$/)
head, start, year, holder = year_match.captures
unless (year == cur_year.to_s)
markdown(
":warning: The year in this license header is outdated, time to update!\n\n",
file: file, line: line_num
)
end
# either way, we return 'true' to stop looking after the first 'match'
true
end
end
end
end
16 changes: 16 additions & 0 deletions danger/premerge-checks.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# SPDX-FileCopyrightText: 2022 Oxhead Alpha
# SPDX-License-Identifier: LicenseRef-MIT-OA

# Checks that are fine to fail during development, but must be fixed before merging.

require_relative 'helpers'

# Fixup commits
if git.commits.any? &:fixup?
fail "Some fixup commits are still there."
end

# Work-in-progress commits
if git.commits.any? &:wip?
fail "WIP commits are still there."
end
Loading

0 comments on commit d259a3c

Please sign in to comment.