Last modified: 2014-04-14 21:13:28 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T65805, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 63805 - MediaWiki core should pass jshint
MediaWiki core should pass jshint
Status: RESOLVED WORKSFORME
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.23.0
All All
: Unprioritized normal (vote)
: ---
Assigned To: Nobody - You can work on this!
https://gerrit.wikimedia.org/r/#/c/12...
:
Depends on:
Blocks: jshint
  Show dependency treegraph
 
Reported: 2014-04-11 05:24 UTC by Nemo
Modified: 2014-04-14 21:13 UTC (History)
5 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Nemo 2014-04-11 05:24:09 UTC
Rillke wrote:
> Build errors unrelated.
> They're related to I24a0d1677f8870970 and the "assert.throws" not being supported in some older browsers (should they run the unit tests?)
Comment 1 Derk-Jan Hartman 2014-04-11 12:59:05 UTC
OK, interesting.

These messages by jshint are informationals, but our jslint parsing script does not make a distinction between errors, warnings and informationals.

It seems that this informational was also recently removed from jshint 2.4.4:
https://github.com/gruntjs/grunt-contrib-jshint/issues/152
https://github.com/jshint/jshint/releases/tag/2.4.4

I have switched these cases back to es3 mode.
Comment 2 Gerrit Notification Bot 2014-04-11 13:03:41 UTC
Change 125397 had a related patch set uploaded by TheDJ:
Switch a few test cases back into es5 mode

https://gerrit.wikimedia.org/r/125397
Comment 3 Krinkle 2014-04-11 23:10:22 UTC
MediaWiki core has passed jshint for over a year. This bug seems invalid or based on a misunderstanding.
Comment 4 Krinkle 2014-04-11 23:14:09 UTC
I think this was created because of the jshint error reported on this change:

 https://gerrit.wikimedia.org/r/#/c/125337/

That change didn't change javascript files, so the error must've been in mediawiki-core and thus one would assume therefore MediaWiki core master is not passing jshint.

However that is not the case.

The change in question was submitted by a user not in the CR whitelist, and thus the non-executive version of jshint was run instead of the one we run on merge.

Our main version has been upgraded to 2.4.x a while back and that added an option for es3/es5. The old version does not understand this option and thus treated files with this option as an error.

The V+1 checkpieline's jshint has been upgraded since (did that yesterday), and this error no longer appears.

Either way, the pipeline that ran on CR+2 never had this error and thus master was always passing.
Comment 5 Nemo 2014-04-12 14:14:58 UTC
Sorry Krinkle, I read the comment above three times but I have no idea what it means.

Are you saying that it's expected to have jenkins-bot place V-1 on a change without errors?
Comment 6 Rainer Rillke @commons.wikimedia 2014-04-12 19:04:25 UTC
(In reply to Nemo from comment #5)
> The V+1 checkpieline's jshint has been upgraded since (did that yesterday), and > this error no longer appears.
So I guess users not in the Code Review (CR) whitelist (where's that?) are no longer getting "build failed".

> However that is not the case.
This is just a friendly way of Krinkle explaining that I my guess was totally wrong.
Comment 7 James Forrester 2014-04-14 21:13:28 UTC
(In reply to Rainer Rillke @commons.wikimedia from comment #6)
> So I guess users not in the Code Review (CR) whitelist (where's that?) are
> no longer getting "build failed".

In the config for Zuul, the glue between gerrit and Jenkins:

https://git.wikimedia.org/blob/integration%2Fzuul-config.git/HEAD/layout.yaml#L94

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links