[mozlint] Improve the default behaviour if no file paths are specified
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement, P2)
Tracking
(firefox88 fixed)
Tracking | Status | |
---|---|---|
firefox88 | --- | fixed |
People
(Reporter: ahal, Assigned: nisargkarun1997)
References
Details
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
Comment 3•7 years ago
|
||
Updated•7 years ago
|
Updated•7 years ago
|
Reporter | ||
Comment 4•5 years ago
|
||
Support files have been supported for a long time... we should really fix this :). Options for a default include:
--outgoing
--outgoing --workdir
*
(lints same files as current default but does so with more parallelization)
Reporter | ||
Comment 5•5 years ago
|
||
I'm on the fence about which default is better. I think 1) or 2) will solve the most common cases (lint the things that I touched) and will also be the fastest. Though it might lead to some suprising UX. E.g, if a developer has mach
on their path, they could do:
$ cd devtools
$ mach lint
And be surprised that things outside of devtools
were linted. So 3) seems like it would provide the most consistent command line UX. It also provides backwards compatibility in case anyone was invoking mach lint
from a script/editor/etc.
Comment 6•5 years ago
|
||
I'm on the fence, in some ways I think 3) would result in the least surprises for developers. Generally if we run a command like that, we expect it to apply to the whole directory (even if that's madness for all of lint).
If we did one of the others, I'd vote for 2) since I think that makes most sense.
If we defaulted to outgoing we could get it to print out the fact it is doing outgoing, and say how to override it. Possibly ./mach eslint
would still default to the whole tree in that case, as it isn't as bad as ./mach lint
on the whole tree, and probably intentional.
Reporter | ||
Comment 7•5 years ago
|
||
Ok yeah, I think it's best to go with 3) for now. We can always switch it later if we find there's a need. I'll see if I can get this fixed sometime during the all-hands.
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Reporter | ||
Comment 9•5 years ago
|
||
In bug 1621322, :sfink was surprised that running mach lint
without arguments didn't lint his touched files. I think we should reconsider here and run --outgoing --workdir
by default.
One possible hybrid is to only use this behaviour if we're running from the root. So e.g,:
$ cd topsrcdir
$ mach lint # only lints outgoing/workdir
$ cd devtools
$ mach lint # lints entire devtools directory
Not sure if this makes things more or less confusing.. but a blank mach lint
from the root of the repo is completely unusable, so IMO confusing behaviour is better than the status quo.
Reporter | ||
Comment 10•4 years ago
|
||
So to recap we should lint outgoing/workdir when all of the following are true:
- No paths or linters were specified (i.e we are running all linters on all paths)
- CWD == lint.root
- self.vcs is not None
When we do this substitution, we should also print a warning explaining what is happening and how to work around it (e.g by passing in .
)
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D105115
Comment 12•4 years ago
|
||
Comment 13•4 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•