Fixes T6060. If a dependency of an ArcanistExternalLinter subclass is missing, emit a warning but continue the linting process as much as possible. This is generally a more reasonable behaviour.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T6060: Arcanist should skip linters that have missing dependencies
> arc lint -- some/file.js [2014-09-10 13:50:32] PHLOG: 'Unable to locate binary "jshint" to run linter ArcanistJSHintLinter. You may need to install the binary, or adjust your linter configuration. TO INSTALL: Install JSHint using `npm install -g jshint`.' at [/home/joshua/workspace/github.com/phacility/arcanist/src/lint/engine/ArcanistLintEngine.php:318] OKAY No lint warnings
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- master
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 2490 Build 2494: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
One concern that I have is that this could cause diffs to show up as "Lint OK" in differential, even if the diff would've failed linting had the dependencies been correctly installed. Perhaps we need a new "Partially linted" status?
Comment Actions
I think that we should allow linting to "skip" specific linters in the same way that unit testing allows for. Any linters that are skipped due to missing dependencies can then show up as skipped in the differential UI.
If this sounds reasonable then im happy to send a diff for this (if you know roughly what is involved and could elaborate that would be great too)
Comment Actions
Yeah, I'm onboard with skipping and think that's the approach we should pursue. I don't think there are any real traps.