Page MenuHomePhabricator

fail on whitespace error in submitted files
Closed, ResolvedPublic

Description

We try to avoid trailing whitespaces in files but do not enforce it for now. git comes with a whitespace checker : git diff --check, which will fail whenever files in the commit introduce trailing spaces.

We need to run 'git diff --check' in the universal linter.


Version: unspecified
Severity: enhancement

Details

Reference
bz42628

Event Timeline

bzimport raised the priority of this task from to Normal.Nov 22 2014, 1:11 AM
bzimport set Reference to bz42628.
hashar created this task.Dec 2 2012, 5:17 PM

For Mediawiki core and extensions: https://gerrit.wikimedia.org/r/37803

I have amended above change which was faulty and also forced color usage. An example console output is https://integration.mediawiki.org/ci/job/mediawiki-core-lint/2204/console (link not permanent) which show:

14:26:57 + git --work-tree=/var/lib/jenkins/jobs/mediawiki-core-lint/workspace diff --color --check 'HEAD^..HEAD'
14:26:57 whitespace.php:2: trailing whitespace.
14:26:57 +echo "I got trailing space";XXXXX
14:26:57 Build step 'Execute shell' marked build as failure
14:26:57 Finished: FAILURE

(where XXXXX is a placeholder for 5 whitespaces and are shown red in the console).

Have applied it in production to the mediawiki-core-lint job. Will deploy for other repositories later on.

I have removed the whitespace check on 'mediawiki-core-lint' since there are some case where we might actually need trailing whitespaces.

We want the whitespace check to be a different Jenkins job that would not block on failure (non-voting in Zuul).

hashar added a comment.Jan 7 2013, 6:58 PM

https://gerrit.wikimedia.org/r/#/c/37803/ does whitespace check on mediawiki/core and mediawiki extensions

hashar added a comment.Jan 7 2013, 9:23 PM

Raymond reported to me an issue with a legitimate change that got rejected because of the whitespace check : https://gerrit.wikimedia.org/r/#/c/42601/ that introduce a message file with a trailing whitespace :-]

https://gerrit.wikimedia.org/r/42672 moves the whitespace check of the -lint jobs
to new -whitespaces jobs which can then be made non voting in Zuul.

hashar added a comment.Jan 7 2013, 9:41 PM

I have made whitespace jobs to be non voting by default : https://gerrit.wikimedia.org/r/42677

Then added a whitespace check in mediawiki core with https://gerrit.wikimedia.org/r/42678

Can this also fail if a PHP file starts with whitespace? Let me know if you prefer I file a separate bug.

We have the whitespace job in Jenkins now though it does not vote because there is a lot of corner case where it would provide false positives. So that is just giving a hint :)