Details
- Reviewers
joshuaspence epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T6311: 'arc linters' no longer reports linters as 'Configured', all show as 'Available'
- Commits
- rARC6f7bedaceb68: Fix linter config check
Diff Detail
- Repository
- rARC Arcanist
- Branch
- arcpatch-D10773
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 3524 Build 3532: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
I initially attempted to implement canRun(), but if it's false, the lint engine just ignores the linters. I think it should fail the lint instead.
src/lint/linter/ArcanistExternalLinter.php | ||
---|---|---|
358 | I think that returning null would make more sense. |
I think you missed something when you updated the diff? It looks exactly the same to me.
I was trying to update it to use ArcanistUsageException, and didn't notice how D11205 hasn't landed yet, and things got weirder from there.
Like, php is fine with trying to catch an exception class that was never defined.
OTOH, once the exception we're catching there is ArcanistUsageException, the whole try-catch-checkBinaryConfiguration block is readable enough, IMO. I'll update it, once it's actually landed...
Bumping this seeing as it fixes T6311: 'arc linters' no longer reports linters as 'Configured', all show as 'Available'.
src/lint/linter/ArcanistExternalLinter.php | ||
---|---|---|
355–359 | I think that narrowing the caught exception here resolves these two. |
@epriestley, it would be great to land this. Without this fix, arc linters is a bit broken.