ci: Add checks for banned keywords in merge requests

Using the same approach as we have for code style checks (the
`style-check-diff` CI job), check the diff for any banned keywords like
‘TODO’, and also check the commit messages.

The keyword ‘TODO’ is often used by developers to indicate a part of a
commit which needs further work, and hence which shouldn’t yet be merged.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

Fixes: #1551
This commit is contained in:
Philip Withnall 2019-04-25 11:23:26 +01:00
parent 5d32b99d0c
commit 3f51963b2d
3 changed files with 113 additions and 0 deletions

View File

@ -37,6 +37,14 @@ style-check-diff:
script:
- .gitlab-ci/run-style-check-diff.sh
check-todos:
extends: .only-default
image: $DEBIAN_IMAGE
stage: style-check
allow_failure: true
script:
- .gitlab-ci/run-check-todos.sh
fedora-x86_64:
extends: .build
image: $FEDORA_IMAGE

89
.gitlab-ci/check-todos.py Executable file
View File

@ -0,0 +1,89 @@
#!/usr/bin/env python3
#
# Copyright © 2019 Endless Mobile, Inc.
#
# SPDX-License-Identifier: LGPL-2.1-or-later
#
# Original author: Philip Withnall
"""
Checks that a merge request doesnt add any instances of the string todo
(in uppercase), or similar keywords. It may remove instances of that keyword,
or move them around, according to the logic of `git log -S`.
"""
import argparse
import re
import subprocess
import sys
# We have to specify these keywords obscurely to avoid the script matching
# itself. The keyword fixme (in upper case) is explicitly allowed because
# thats conventionally used as a way of marking a workaround which needs to
# be merged for now, but is to be grepped for and reverted or reworked later.
BANNED_KEYWORDS = [
'TO' + 'DO',
'X' + 'XX',
'W' + 'IP',
]
def main():
parser = argparse.ArgumentParser(
description='Check a range of commits to ensure they dont contain '
'banned keywords.')
parser.add_argument('commits',
help='SHA to diff from, or range of commits to diff')
args = parser.parse_args()
banned_words_seen = set()
seen_in_log = False
seen_in_diff = False
# Check the log messages for banned words.
log_process = subprocess.run(
['git', 'log', '--no-color', args.commits + '..HEAD'],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8',
check=True)
log_lines = log_process.stdout.strip().split('\n')
for line in log_lines:
for keyword in BANNED_KEYWORDS:
if re.search('(^|\W+){}(\W+|$)'.format(keyword), line):
banned_words_seen.add(keyword)
seen_in_log = True
# Check the diff for banned words.
diff_process = subprocess.run(
['git', 'diff', '-U0', '--no-color', args.commits],
stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding='utf-8',
check=True)
diff_lines = diff_process.stdout.strip().split('\n')
for line in diff_lines:
if not line.startswith('+ '):
continue
for keyword in BANNED_KEYWORDS:
if re.search('(^|\W+){}(\W+|$)'.format(keyword), line):
banned_words_seen.add(keyword)
seen_in_diff = True
if banned_words_seen:
if seen_in_log and seen_in_diff:
where = 'commit message and diff'
elif seen_in_log:
where = 'commit message'
elif seen_in_diff:
where = 'commit diff'
print('Saw banned keywords in a {}: {}. '
'This indicates the branch is a work in progress and should not '
'be merged in its current '
'form.'.format(where, ', '.join(banned_words_seen)))
sys.exit(1)
if __name__ == '__main__':
main()

16
.gitlab-ci/run-check-todos.sh Executable file
View File

@ -0,0 +1,16 @@
#!/bin/bash
set +e
# We need to add a new remote for the upstream master, since this script could
# be running in a personal fork of the repository which has out of date branches.
git remote add upstream https://gitlab.gnome.org/GNOME/glib.git
git fetch upstream
# Work out the newest common ancestor between the detached HEAD that this CI job
# has checked out, and the upstream target branch (which will typically be
# `upstream/master` or `upstream/glib-2-62`).
# `${CI_MERGE_REQUEST_TARGET_BRANCH_NAME}` is only defined if were running in
# a merge request pipeline; fall back to `${CI_DEFAULT_BRANCH}` otherwise.
newest_common_ancestor_sha=$(diff --old-line-format='' --new-line-format='' <(git rev-list --first-parent upstream/${CI_MERGE_REQUEST_TARGET_BRANCH_NAME:-${CI_DEFAULT_BRANCH}}) <(git rev-list --first-parent HEAD) | head -1)
./.gitlab-ci/check-todos.py "${newest_common_ancestor_sha}"