Change 574202 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Maintenance: eslint (run with: grunt test) will automatically fix trivial coding style errors

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

Change 574203 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Maintenance: grunt test won't run the unnecessary 'svgmin' task. grunt (default) will, though.

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

Change 574223 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Maintenance: add grunt lint and grunt minify, consistent with core, and grunt build, npm build, remove grunt default

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

@Esanders wrote on gerrit:
Oh I see you added this in this commit. We don't run 'fix' by default, some fixes (especially whitespace fixes) require manual checking, so shouldn't be run without the user knowing about it. If we want to change this convention we should have a separate discussion about it.

@Demian wrote:
Yes, it saved me maybe hours. It's a good point tho to make the user aware. Can we do something about that, maybe ask the user interactively?
What whitespace fix needs manual checking? Can you give an example?

I found the fix option very useful, unfortunately it's not possible to specify on the command line afaict. A separate task, maybe 'lintfix' or 'fix' (or sg short) could do that, so the user requests it explicitly.

In the past we have sometimes created a subtask for fix, then you can grunt eslint:fix but it might be nicer to use grunt.option('fix'):

eslint: {
  options: {
    fix: grunt.option('fix')
  }
}

then
grunt eslint --fix

Another issue I just noticed is that if you run fix as part of default then it will get run by default by CI and errors will not get detected.

In T245930#5912051, @Esanders wrote:

Another issue I just noticed is that if you run fix as part of default then it will get run by default by CI and errors will not get detected.

I've removed that from the chain as WIP, so the rest is not stalled.

Weird: lint won't report this line, but --fix will add 2 spaces: https://github.com/wikimedia/mediawiki-extensions-MultimediaViewer/blob/master/resources/mmv/mmv.mixins.less#L23
I wonder why. This is a line from 2016.

Also, some upstream stylelint bugs for reference (not a concern of this task):
T245957: [Development tooling] grunt stylelint gives false positives inside comments, attributed to the wrong line number (tracking upstream reports)

Change 574486 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Maintenance: grunt jsfix, grunt cssfix and their shorthands grunt jf, grunt cf automatically fixes trivial coding style errors

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

Change 574490 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Maintenance: grunt stylelint --fix added a 2 spaces

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

Change 574203 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Maintenance: grunt test won't run the unnecessary 'svgmin' task. grunt (default) will, though.

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

Change 574223 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Maintenance: add grunt lint and grunt minify, consistent with core, and grunt build, npm build

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

Change 574202 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Maintenance: grunt lint --fix, grunt eslint --fix, grunt stylelint --fix automatically fixes trivial coding style errors

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

The future for the above patches: T242781: Transition Gruntfile.js tasks to NPM scripts (Vector)

@Esanders I've submitted a POC patch in T242781 that makes it possible to delete the 40+ MiB node_modules (including grunt) from the project folder.

Per a comment on gerrit, an example where auto-fixing needs manual review is the auto-indenting of block comments:

( function () {
/**
 * Check if this thumbnail should be handled by MediaViewer
 *
 * @param {jQuery} $thumb the thumbnail (an `<img>` element) in question
 * @return {boolean}
 */
MMVB.isAllowedThumb = function ( $thumb ) {
	...
};
}() );

( function () {
	/**
 * Check if this thumbnail should be handled by MediaViewer
 *
 * @param {jQuery} $thumb the thumbnail (an `<img>` element) in question
 * @return {boolean}
 */
	MMVB.isAllowedThumb = function ( $thumb ) {
		...
	};
}() );

Good to know, I'll pay attention to that. Thank you.

Change 574636 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Replace grunt tasks with npm scripts, remove unnecessary devDependencies.

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

Change 574490 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Maintenance: npm -s run lint:fix caught some missed empty lines and spaces.

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

Change 575396 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Regenerate 'package-lock.json'

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

Change 575664 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Update node_modules dependencies in package-lock.json

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

Change 575665 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Replace grunt* devdependencies with eslint, update node_modules dependencies in package-lock.json

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

Change 575666 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Regenerate package-lock.json

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

Change 575668 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] Update node_modules dependencies in package-lock.json after deleting node_modules

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

Change 637820 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/MultimediaViewer@master] [cleanup] Fix eslint (new version) warnings

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

Change 637820 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] [cleanup] Fix eslint (new version) warnings

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