green, red, and white high voltage circuit breaker

Automate code-review chores with a simple Dangerfile

One of the most annoying chores in code-reviewing is having to say: “You forgot to [unwritten rule]”. It feels arcane, it annoys the code-reviewer and the guy doing the work, it builds frustration, sparks useless arguments, some edging on the brink of flame wars. Well, not unless the rules are written, the rules themselves codereviewed and agreed upon in the team. One idea to automate-away some of the most common concerns and focus on the interesting logic is to automate the chores away using the Danger bot.

In one work situation, I had the possibility to implement the first pipelines the team has used and to provide some standardization going forward. Our Git-flow evolved around 3 branches (integration, dev and master) with the direction of code flowing being from left to right, passing through QA as it went from integration to “development” branch (tied to the specific cloud account) and on to “master” from which release branches were triggered, later to be deployed on the production cloud account. The existence of the “integration” branch basically is to check-in the code somewhere until we are ready to “cut the cord” and release a new version for QA and extensive development testing (with custom data and a sample of anonymous production data).

While the pipelines we’ve wrote are a tad more complex than what we showcase today, since we’ll be focusing only on the Dangerfile, we’ll first just show the code (in Ruby) and we’ll detail later in exactly what it does. Note that our pipelines run on Gitlab, so most of the code below is Gitlab specific. Feel free to check the Dangerfile reference for other SCM systems.

# Requirements from CI environment
project_id = ENV['CI_PROJECT_ID']
merge_request_id = ENV['CI_MERGE_REQUEST_IID']

# Detections
has_docs_update = !git.modified_files.grep(%r{src/docs}).empty?
has_opened_mr_against_standard_branches = ["integration", "dev", "master"].include? (gitlab.branch_for_merge)
has_assignees = gitlab.mr_json["assignee"]
has_blocking_discussions_resolved = gitlab.mr_json["blocking_discussions_resolved"]
known_contributors = gitlab.api.contributors(project_id, { order_by: 'commits', sort: 'desc'})

# Default messages
message "Thank you @%s for the contribution! ❤️ Note that automated Checkstyle + Tests + SonarQube must pass for the contribution to be then reviewed 🤓" % gitlab.mr_author
failure "Please re-submit this MR to integration branch, the others being used for CI automation 😇" unless has_opened_mr_against_standard_branches
(warn "Make sure this MR has assignees set for review in order for it to be accepted and merged 🙏 Suggestions: %s" % known_contributors.map {|contrib| contrib["name"]}.first(3).join(', ')) unless has_assignees
warn "Make sure this MR has updates to documentation in src/docs 🙏 as it gets published automatically to Confluence 💪" unless has_docs_update
warn "Make sure open discussions 😞 on this MR are resolved. Discuss (or negotiate in 🍻) with your peers to solve them 🤗" unless has_blocking_discussions_resolved

# Can't humanly review more than > 250 lines of code (with effort)
# Inspired by @ https://medium.com/@hugooodias/the-anatomy-of-a-perfect-pull-request-567382bb6067
fail "Your MR has over 500 lines of code 😱 No living human being can review that. Try to split 👍" if git.lines_of_code > 500

# Warn on all checks, instead of failing
commit_lint.check warn: :all

# Have todos fixed
todoist.message = "Temporary TODOS are permanent ones, you can't fool us, so, please, fix them 🙉"
todoist.warn_for_todos
todoist.print_todos_table

# Check for various flows or labels
has_qa_required = (gitlab.branch_for_head == "integration") && (["dev", "master"].include? (gitlab.branch_for_merge))

# Labels
labels = [ "mr::status/new" ]

if has_qa_required == true
    labels.append ("qa::required")
end

if has_docs_update == false
    labels.append ("docs::required")
end

# Set some IDs
approver_groups= []
approver_groups.append (123456789) # Your own Gitlab/QA group

# Update the MR with all determined labels
gitlab.api.update_merge_request(project_id, merge_request_id, { labels: labels.join(',') })

So what happens above basically? First of all we check for updates to src/docs which we publish later to Confluence, based on Asciidoc plus the docToolChain tooling. We then check that the MR was opened only against the 3 standard branches thus ensuring feature branches flow normally towards the integration branch before being promoted to development and production later on. We then check for assignee as any MR should have someone assigned to it. We also check for open discussions to be resolved and we suggest some contributors based on recent Git history of the project.

If the MR is too big, above 500 lines of code, we let the user know it’s quite heavy if not impossible to properly review, though we don’t fail the pipeline for that. Using the commit_lint plugin we check for proper commit messages (most of which are not) and we check for leftover TODOs in the committed source code, just to have the developer take a 2nd look at the implementation before checking it in.

Finally, our workflow also requires QA to approve the building of the artifacts and promotion to both “development/qa” environments and then on to production. Thus, if the MRs are made against the trigger branches (dev/master) then the QA label that the team is following is attached so that the QA guys get a notification (if they’re subscribed to the Gitlab label of course).