The documentation vaguely suggests that this is the intended behavior.
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Required Signatures
L28 Phacility Individual Contributor License Agreement
None
Diff Detail
- Repository
- rARC Arcanist
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 8973 Build 10534: arc lint + arc unit
Event Timeline
@csilvers, I think you are the flagship user of this binding -- are these changes going to break you?
The documentation vaguely suggests that this is the intended behavior.
I'd probably didn't intend this, can you point me at where things are ambiguous in your reading? We can clarify the docs, at least.
I think the default behavior of not linting this stuff is mostly-reasonable and probably correct -- many linters do not work correctly when run as linter file-that-does-not-exist or linter directory, and these cases are difficult to exclude with .arclint rules. I'm tentatively OK with adding a script-and-regex.lint-everything configuration option which defaults to false but causes the behavioral changes here when enabled, although this is a bit of a slippery slope toward script-and-regex.lint-just-this-subset-of-things which I'm less-than-enamored by the prospect of. But I'm OK with an option + a documentation update.
The documentation contains this paragraph,
Note that when run via arc diff, the list of files to be linted includes deleted files and files that were moved away by the change. The linter should not assume the path it is given exists, and it is not an error for the linter to be invoked with paths which are no longer there. (Every affected path is subject to lint because some linters may raise errors in other files when a file is removed, or raise an error about its removal.)
I liberally read this to mean that the linter should be run against all files. Indeed clarifying the documentation here would be quite useful; I killed quite a long time today trying to work out why my linter wasn't getting run.
However, at this point I have no need for this change as we'll be using the extension originally proposed for merging in D14535.
Ah, thanks! I'll fix the docs, I think they probably predate us breaking out these special filetypes (long ago, every linter needed to account for them, but most linters did not and could not easily be made to, and it was easy to not test these special cases, and very few linters care about deleted files anyway).
If you don't need this anymore, let's just leave things as they are.
Sounds like this ended up having no changes? Let me know if you still need input from me for anything.