Last modified: 2014-03-27 16:39:28 UTC
The jslint jobs are being run under /srv/ssd/jenkins-slave/workspace . On the gallium host /srv is also a checkout of integration/docroot.git which contains .jshint and .jsignore The result is that whenever a repo does not provide .js* files, it ends up reusing the one from the CI docroot. The workaround is to provide a nill .jshintrc like have been done on labs/tools/grrrit https://gerrit.wikimedia.org/r/#/c/77309/
The fix would be to add a dummy .jshintrc in integration/docroot.git at /srv/ssd/jenkins-slave/workspace/.jshintrc . That should reset the parameters Jenkins jobs.
No need for workaround, projects are expected to have their jshintrc settings in their own repository. Changing then in any other place will cause disruption as: * When the defaults change, repositories need to update their code to conform to the new settings. If it inherits from something (e.g. our own defaults, or the jshint defaults or whatever else) this will inevitably cause jenkins jobs to fail unexpectedly for existing repositories after an upgrade eventhough nothing changed on their part. This will eg. cause patches that were passing to suddenly now fail. * Makes it hard or impossible to apply settings locally in development environment (e.g. using your editor's jshint plugin, or when using grunt or node-jshint from the command line, it wouldn't have the same settings). If we're going to make an effort to do anything about the false positives for jobs that someone has incorrectly enabled a jshint job for without there being a proper jshintrc file, then I'd recommend spending that effort on making the job fail if the repo doesn't have a jshintrc file.
See also https://www.mediawiki.org/wiki/CC/JS#Linting
So I would much prefer we move the .jshintrc somewhere else or provide a default .jshintrc in the workspace repository. I just came across a case in which jslint job yields two different results depending on the slave it ran on: Job https://gerrit.wikimedia.org/r/#/c/115214/ modify some javascript lanthanum pass https://integration.wikimedia.org/ci/job/mwext-CirrusSearch-jslint/2/console gallium fails https://integration.wikimedia.org/ci/job/mwext-CirrusSearch-jslint/3/console
The job should not be enabled (especially not voting) on repositories without a jshintrc file. This isn't a question of providing a sensible default. Enabling it without a jshintrc file will always fail, and if not, it will be a false positive. Stop enabling (or honouring requests to do so) for repositories without such a file in place. I've said it before but I'll say it again, creating an empty .jshintrc file is also not a solution and I'll hereby declare it an anti-pattern. Stop doing it please. Repositories may (not must) inherit the Wikimedia coding conventions[1]. However projects usually require some additional configuration specific to their poject. Providing a default file in the directory chain with the Wikimedia code conventions is not a solution either for the same reason using other defaults is not a solution. The settings must ship with the repository. 1) The repository is not dependent on our infrastructure (running jshint or grunt locally should have the same effect, and linting plugins for code editors need access to the settings in the expected location in order to give the right feedback). 2) Builds don't unexpectedly start breaking. When the defaults change (either the one in docroot, the jshint default or ours), this will break peoples tests for no reason and with no obvious source of the problem. When our conventions are adjusted, these changes should propagate to all the projects. However having these settings propagate automatically is not productive because projects need to actually update their code to account for the new settings, and until they do so, it's not very productive to break their build because of a minor coding style change. Instead they can update it whenever (and if) they want to. And again, reason #1. Closing as INVALID. If changes in your repository produce broken builds because of missing or invalid jshint settings, please blame whoever enabled it without ensuring a jshintrc file is in place for enabling a useless job that is bound to fail, and then add a jshintrc file[2] to the repository so the build will start working :) [1] https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript [2] https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Linting PS: How about we wrap bin/jshint in a script that returns early if .jshintrc is missing?
Sorry Timo but I am reopening this bug. A typical use case is: * someone write some js code in a repository not having a .jshintrc * he runs jshint (i.e. with defaults) and iterate until the code pass * change is pushed to Gerrit, a job is run on one of the slaves which is not gallium * someone vote +2 which trigger the job on gallium => gating fail with unexpected jshint errors that user can not figure out I will just add some default jshintrc to the docroot to fix that
Change 119750 had a related patch set uploaded by Hashar: contint: override .jshintrc file on gallium https://gerrit.wikimedia.org/r/119750
OK. I agree. An empty file as default in our infrastructure is useful. That way default behaviour in Jenkins will match default behaviour for developers using jshint locally. However we should not create empty files in regular code repositories, and I suppose we won't have to with this puppet change. Thanks.
Change 119750 abandoned by Hashar: contint: override .jshintrc file on gallium Reason: So apparently we should get the web docroot under /srv/www to avoid conflicting with docroot. That needs a lot of changes in Apache config and Jenkins jobs to adjust the paths. I don't think it is worth the effort so abandoning this patch. https://gerrit.wikimedia.org/r/119750
Abandoning, feel free to take over I have better things to handle.
