Page MenuHomePhabricator
Create Task
Maniphest T179195

Replace use of .eslintignore with ignore in Gruntfile.js in all repos, for consistency
Closed, DeclinedPublic
Actions

Description

The following skins and extensions have a .eslintignore file, but it also possible to exclude the vendor and node_modules folder over grunt config in Gruntfile.js

I would say it is better to use Gruntfile.js to see the ignore at one place, because jsonlint or other build tools are using it too.
Pro argument is that IDE can use the file (I have no idea, if there is any IDE support for eslint)

It is okay to use eslintignore or should we decide to use Gruntfile.js? That would remove a file from the root path and define a default handling.

Thanks for comments

This could also be done for .jshintignore or other tools over grunt.

Skins
  • Timeless
Extensions
  • ArticlePlaceholder
  • PropertySuggester
  • Wikibase
  • WikibaseLexeme
  • WikibaseMediaInfo
  • WikibaseQuality
  • WikibaseQualityConstraints
  • WikibaseQualityExternalValidation
  • Wikidata.org
  • WikimediaBadges

libs

  • data-values/value-view - gerrit
  • wikibase/javascript-api - gerrit
  • wikibase-serialization - rWBSERJS
  • datavalues-javascript - rDVJS
  • wikibase-data-model - rWBDMJS

Details

Show related patches Customize query in gerrit

Related Objects

Event Timeline

Umherirrender created this task.Oct 27 2017, 5:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 27 2017, 5:26 PM
Florian added a comment.Oct 28 2017, 1:03 PM

The pro argument is (like you said already): all the exclusions can be found in one file (as long as it integrates with grunt). That would mean, that it would be easier to see, what files will be checked with the tool (you do not need to open Gruntfile.js to see what is _included_ and then look for a specialized file in the projects root to see what is _excluded_).

The contra argument I can think of: One could assume, that excludes are defined in a separate ignore file, if no ignore file is present, they could assume, that no files are ignored. However, this argument can most likely be fixed when we agree on using Gruntfile.js and after some time it will be something that is "well-known", I think.

However, personally I do not have a strong opinion about what way to use, even if I kind of like to do not have so much .*ignore files in the projects root :) That are my 2 cents :P

thiemowmde triaged this task as Low priority.Oct 28 2017, 2:26 PM
thiemowmde added projects: Wikidata, patch-welcome.
thiemowmde added a subscriber: WMDE-leszek.

I vote for removing the separate .eslintignore files for the given reasons. Personally, I prefer having less clutter in the root directories of these projects. I wish the exclusions are closer to the rest of the configuration, either in the .eslintrc.json file (if this is possible), or in Gruntfile.js, as discussed.

I would love to hear @WMDE-leszek's opinion. If he agrees (or does not care) I would say go for it. I will then merge such patches.

WMDE-leszek added a comment.Oct 29 2017, 10:44 AM

So my two cents below. Please keep in mind that I don't have much experience in JS development, so I might be saying something which is complete rubbish.

  • As far as I understand, using .eslintignore file is a standard and kind of "native" way of defining paths to ignore used by eslint. It is also commonly in numerous JS projects I've seen around.
  • I am not entirely convinced that having a part of eslint config in .eslintrc.json, and then part (ignore paths) in grunt file is actually making things more clear.
  • Moving this to gruntfile would be "binding" (part of) esling config to a particular task runner. If developers of an extension, or WMF as a whole, decided to move to another JS task runner, all this information would also need to migrated etc. Not that it something which would be a rocket science, it is just making configuration stored in two different places and formats (as said in the point above).
  • I can totally understand preferring to have ignore path defined in a single place. On the other hand, I could also imagine the gruntfile might become cluttered if all kind of stuff is moved in there. It might be harder to find the relevant important information if there all kind of minor config also mashed in.

Overall: I have no strong preference on either option. Also, it is clear to me, all "arguments" made by me are really minor stuff. That said, from my perspective I also don't see a reason to "enforce" one or either way on extension/skin authors. This kind of thing seem to me to be something each extension devs could do whatever way they prefer, as long as both way are actually quite "standard" and nothing super weird that would be confusing to outside folks. I wouldn't oppose any decision, though, if this was something which is generally preferred :)

gerritbot subscribed.Apr 20 2018, 8:13 PM

Change 427997 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/ArticlePlaceholder@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a project: Patch-For-Review.Apr 20 2018, 8:13 PM
gerritbot added a comment.Apr 20 2018, 8:20 PM

Change 428005 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/PropertySuggester@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 8:59 PM

Change 428009 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/Wikidata.org@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:00 PM

Change 428011 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikimediaBadges@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:00 PM

Change 428012 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikibaseQualityExternalValidation@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:00 PM

Change 428013 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikibaseQualityConstraints@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:01 PM

Change 428014 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikibaseQuality@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:01 PM

Change 428015 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikibaseMediaInfo@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:01 PM

Change 428016 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/Wikibase@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 20 2018, 9:03 PM

Change 428018 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/WikibaseLexeme@master] Move information from .eslintignore into Gruntfile.js

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

Umherirrender mentioned this in rEWLE889594349f6a: Move information from .eslintignore into Gruntfile.js.Apr 20 2018, 9:04 PM
Umherirrender mentioned this in rEWBIc0c392e99af2: Move information from .eslintignore into Gruntfile.js.Apr 20 2018, 9:09 PM
Umherirrender mentioned this in rEWLE65d7434531c7: Move information from .eslintignore into Gruntfile.js.Apr 20 2018, 9:46 PM
Umherirrender mentioned this in rEWLE3666921fa9f5: Move information from .eslintignore into Gruntfile.js.Apr 20 2018, 10:00 PM
Umherirrender mentioned this in rEWLE07e6b01a6a75: Move information from .eslintignore into Gruntfile.js.Apr 21 2018, 5:44 AM
Umherirrender mentioned this in rEWLEabe55a89add0: Move information from .eslintignore into Gruntfile.js.Apr 21 2018, 6:12 AM
Umherirrender mentioned this in rEWLEf61639c126e0: Move information from .eslintignore into Gruntfile.js.
Umherirrender mentioned this in rEWLEabea90c3fe5d: Move information from .eslintignore into Gruntfile.js.Apr 21 2018, 6:17 AM
gerritbot added a comment.Apr 21 2018, 10:24 AM

Change 428005 merged by jenkins-bot:
[mediawiki/extensions/PropertySuggester@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 21 2018, 10:28 AM

Change 428009 merged by jenkins-bot:
[mediawiki/extensions/Wikidata.org@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 21 2018, 10:29 AM

Change 427997 merged by jenkins-bot:
[mediawiki/extensions/ArticlePlaceholder@master] Move information from .eslintignore into Gruntfile.js

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

Reedy unsubscribed.Apr 21 2018, 10:41 AM
ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-04-24 (1.32.0-wmf.1)).Apr 21 2018, 11:00 AM
gerritbot added a comment.Apr 21 2018, 1:26 PM

Change 428061 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/skins/Timeless@master] Move information from .eslintignore into Gruntfile.js

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

Umherirrender mentioned this in rEWBI99bdbb9778b4: Move information from .eslintignore into Gruntfile.js.Apr 21 2018, 3:41 PM
gerritbot added a comment.Apr 21 2018, 4:26 PM

Change 428061 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Move information from .eslintignore into Gruntfile.js

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

Jdforrester-WMF updated the task description. (Show Details)Apr 23 2018, 4:09 PM
Jdforrester-WMF updated the task description. (Show Details)
Jdforrester-WMF mentioned this in rEWBI265dd71459fc: Move information from .eslintignore into Gruntfile.js.Apr 23 2018, 4:12 PM
gerritbot added a comment.Apr 23 2018, 4:25 PM

Change 428015 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 23 2018, 4:38 PM

Change 428018 merged by jenkins-bot:
[mediawiki/extensions/WikibaseLexeme@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 23 2018, 4:44 PM

Change 428011 merged by jenkins-bot:
[mediawiki/extensions/WikimediaBadges@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 23 2018, 4:44 PM

Change 428014 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQuality@master] Move information from .eslintignore into Gruntfile.js

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

gerritbot added a comment.Apr 23 2018, 4:46 PM

Change 428013 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Move information from .eslintignore into Gruntfile.js

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

Jdforrester-WMF updated the task description. (Show Details)Apr 23 2018, 4:47 PM
Umherirrender updated the task description. (Show Details)Apr 23 2018, 5:59 PM
Umherirrender updated the task description. (Show Details)Apr 23 2018, 6:09 PM
gerritbot added a comment.Apr 23 2018, 6:52 PM

Change 428016 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Move information from .eslintignore into Gruntfile.js

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

Jdforrester-WMF updated the task description. (Show Details)Apr 23 2018, 6:53 PM
Jdforrester-WMF renamed this task from Decide whether we want to use .eslintignore or ignore over GruntFile.js to Replace use of .eslintignore with ignore in Gruntfile.js in all repos, for consistency.Apr 23 2018, 7:13 PM
gerritbot added a comment.Apr 23 2018, 8:25 PM

Change 428012 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityExternalValidation@master] Move information from .eslintignore into Gruntfile.js

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

Jdforrester-WMF updated the task description. (Show Details)Apr 23 2018, 8:53 PM
Jdforrester-WMF removed a project: Patch-For-Review.
Umherirrender mentioned this in rEWLEbddb29a70f56: Final NoteDb migration updates.Jun 9 2018, 11:56 AM
Umherirrender mentioned this in rEWBIf0ce29a5afae: Final NoteDb migration updates.Jun 9 2018, 5:56 PM
Umherirrender mentioned this in T204176: Decide how to configure ESLint rules and ignores.Sep 13 2018, 4:03 PM
Krinkle changed the task status from Open to Stalled.Sep 13 2018, 7:43 PM

I think moving to Gruntfile is a viable option, but it there are other options. If we really want to defocus on tooling integration, we should move eslintrc to Gruntfile as well. Or, keep the integration and keep both config+ignores out of Gruntfile (either in .eslint* files, or in package.json).

See T204176 for details.

Umherirrender mentioned this in T225325: LibraryUpgrader CI normalisation tasks, June/July 2019.Jun 7 2019, 7:56 PM
• Demian mentioned this in T246321: [EPIC] Transition Gruntfile.js tasks to NPM scripts in some projects.Feb 27 2020, 9:17 AM
• Demian mentioned this in T246326: Process template: How to transition Gruntfile.js tasks to NPM scripts.Feb 27 2020, 9:38 AM
Jdforrester-WMF closed this task as Declined.Apr 9 2020, 6:39 PM
Umherirrender mentioned this in rEPS1924d1713e0a: Move information from .eslintignore into Gruntfile.js.May 25 2023, 6:45 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL