Page MenuHomePhabricator

[Tracking] Allow automatic linting fixing and make sure scripts are portable in Windows
Closed, ResolvedPublic

Description

Follow-up to T242781: Transition Gruntfile.js tasks to NPM scripts (Vector)
This ticket catalogs these patches for easy discoverability.

  • Make scripts portable to Windows (576471)
  • Separate scripts npm test and npm run build (576533)
  • Make it possible to automatically fix linting issues with a simple command npm run lint:fix (575809)
  • Make it possible to automatically fix linting issues quicker with specific commands for styles and code: npm run lint:fix:js, npm run lint:fix:styles (577359)
  • Clean up devdependencies (576472)
  • Add SVG minifier script (575811)
  • Re-cruch SVGs (just related - 575812)

Event Timeline

Demian created this task.Mar 5 2020, 4:47 PM
Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptMar 5 2020, 4:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Demian added a comment.EditedMar 5 2020, 6:17 PM

Demian updated the task description. (Show Details)Mar 5 2020, 6:18 PM
Demian updated the task description. (Show Details)
Jdlrobson renamed this task from Transition Gruntfile.js tasks to NPM scripts (Vector) - additional fixes and tasks to [Tracking] Allow automatic linting fixing and make sure scripts are portable in Windows.Mar 5 2020, 6:36 PM
Jdlrobson triaged this task as Low priority.
Jdlrobson updated the task description. (Show Details)Mar 5 2020, 7:32 PM

Change 575809 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add script 'npm run lint:fix' to fix trivial lint errors

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

Change 576533 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Separate scripts npm test and npm run build: 'test' won't build documentation

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

Change 576472 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add necessary devdependency 'eslint', remove not-explicitly-necessary 'stylelint'

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

Change 577359 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add script 'npm run lint:fix:js' and 'lint:fix:styles', remove 'lint:fix'

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

Change 575809 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Add script 'npm run lint:fix' to fix trivial lint errors

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

Change 575811 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add SVG minifier script npm run svgmin and unify SVGO rules to Wikimedia coding conventions

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

Change 576533 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Separate scripts npm test and npm run build, remove npm run doc from test

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

Change 576534 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add linting *.json to script 'npm run lint:js'

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

Change 576534 abandoned by Aron Manning:
Add linting *.json to script 'npm run lint:js'

Reason:
Can be resurrected if *.json linting becomes a requirement

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

Demian updated the task description. (Show Details)Mar 6 2020, 11:33 AM

Change 575811 merged by jenkins-bot:
[mediawiki/skins/Vector@master] build: Add 'svgo', SVG minifier script and unify SVGO rules

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

Demian updated the task description. (Show Details)Mar 14 2020, 11:14 AM
Demian added a comment.EditedMar 14 2020, 11:27 AM

@Jdlrobson the focus of the "eslint" patch has changed to "stylelint" before your review. Please review again.
@Jdrewniak This patch now just removes the "stylelint" devdependency. Please make an acknowledgment of the current status, as you've added it.
Patch 576472 Remove unnecessary devdependency 'stylelint'

@Jdlrobson the one remaining patch is the implementation of your very insightful idea to run JS and style lint fixing separately to save developer time, as seldom are these necessary to be run together.
It replaces the previous patch that you've accepted and asked me to leave the split for a follow-up patch. Here's the patch:
Patch 577359 Add script 'npm run lint:fix:js' and 'lint:fix:styles', remove 'lint:fix'
As you've said you don't expect to be using these, I assume a replace of the original npm script - to keep it lean - is acceptable to you. Thank you for your idea - it did save me dev time - and please merge this patch too.

Change 576472 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Remove unnecessary devdependency 'stylelint'

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

I have other priorities right now but i can look at the fix patch wednesday earliest.

Change 577359 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Add script 'npm run lint:fix:js' and 'lint:fix:styles', remove 'lint:fix'

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

Jdlrobson closed this task as Resolved.Mar 18 2020, 7:32 PM
Jdlrobson updated the task description. (Show Details)

Thanks for opening this task @Demian. The checklist was helpful for tracking the work and understanding the problems being fixed.

Thanks for opening this task @Demian.

Thank you for the feedback and merges.