Page MenuHomePhabricator

mwext-CirrusSearch-whitespaces
Closed, ResolvedPublic

Description

mediawiki/extensions/CirrusSearch triggers mwext-CirrusSearch-whitespaces which does a very basic whitespaces check using git:

git --work-tree="$WORKSPACE" diff --color --check HEAD^..HEAD

The job should be converted to a Docker container.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
greg triaged this task as Medium priority.Nov 29 2018, 4:42 AM

I think it is easier to add this check directly in one of the existing entry point. Namely add it as a script for composer and thus have it run by the CI jobs that run composer test. That is one less more specific job in CI and it is easier/trivial for developers to reproduce the exact same command.

I looked at the history of commits with a tiny script:

#!/usr/bin/python3

import shlex
import subprocess

out = subprocess.check_output(
    shlex.split('git log --first-parent -n 5000 --pretty=format:%H'))
commits = out.decode().split('\n')

errors = 0
for commit in commits:
    cmd = shlex.split(
        'git diff --color --check {commit}^..{commit}'.format_map(locals()))
    try:
        out = subprocess.check_output(cmd, stderr=subprocess.STDOUT)
    except subprocess.CalledProcessError as e:
        print('%s fails white space check:\n%s' % (commit, e.output.decode()))
        errors = errors + 1
        continue

print("Errors: %s. Commits: %s" % (errors, len(commits)))

There are bunch of commits that are not passing the check. The job is non voting.
If there are interest in checking whitepspaces, that should use an existing entry point (composer would work) and be enforced (ie in a voting job).

I am removing the job that is used solely on this repository and adds little value right now.

Change 484385 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Remove CirrusSearch whitespaces job

https://gerrit.wikimedia.org/r/484385

Change 484385 merged by jenkins-bot:
[integration/config@master] Remove CirrusSearch whitespaces job

https://gerrit.wikimedia.org/r/484385