Bug 1433410 (codespell)

Add codespell support for mach lint

RESOLVED FIXED in Firefox 60

Status

enhancement
RESOLVED FIXED
a year ago
a day ago

People

(Reporter: sylvestre, Assigned: sylvestre)

Tracking

(Depends on 2 bugs, Blocks 1 bug)

unspecified
mozilla60
Dependency tree / graph

Firefox Tracking Flags

(firefox60 fixed)

Details

Attachments

(1 attachment)

I am trying to integrate this tool: https://github.com/lucasdemarchi/codespell/

Example:
./mach lint -l codespell tools/lint/docs/linters/eslint-plugin-mozilla.rst tools/lint/eslint/eslint-plugin-mozilla/lib/processors/xbl-bindings.js

For now, --fix isn't managed because of https://github.com/lucasdemarchi/codespell/issues/314
I can ignore the warning but cannot ignore the autofix as it would require to patch codespell itself.
Comment hidden (mozreview-request)
Assignee

Updated

a year ago
Blocks: 1433417

Comment 2

a year ago
mozreview-review
Comment on attachment 8945745 [details]
Bug 1433410 - Add codespell support for mach lint

https://reviewboard.mozilla.org/r/215852/#review221754

::: tools/lint/codespell.yml:5
(Diff revision 1)
> +---
> +codespell:
> +    description: Check code for common misspellings
> +    include: ['.']
> +    exclude: []

We should at least add `third_party` here. Alternatively we could block on fixing bug 1433522 and bug 1375861 first.

::: tools/lint/spell/__init__.py:94
(Diff revision 1)
> +    if not binary:
> +        print(CODESPELL_NOT_FOUND)
> +        if 'MOZ_AUTOMATION' in os.environ:
> +            return 1
> +        return []

This is fine for now because there's precedent.. but at some point we'll need to make bootstrapping these  things easier.

Maybe with a bit of refactoring we can import `mozboot` (the library behind |mach bootstrap|) and re-use the code in there to get things installed.

::: tools/lint/spell/__init__.py:103
(Diff revision 1)
> +        return []
> +
> +    config['root'] = lintargs['root']
> +    cmd_args = [binary,
> +                '--disable-colors',
> +                # Silent some warnings:

nit: Silence
Attachment #8945745 - Flags: review?(ahalberstadt) → review+
Comment hidden (mozreview-request)

Comment 4

a year ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e414096f1c3b
Add codespell support for mach lint r=ahal
Comment hidden (mozreview-request)

Comment 7

a year ago
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3eefd4e1087c
Add codespell support for mach lint r=ahal

Comment 9

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3eefd4e1087c
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60

Comment 10

a year ago
Can we have a dev-platform+fx-dev post about this? I was confused when I suddenly got warning messages about codespell ("Unable to locate codespell, ...") when using `hg commit`.
Flags: needinfo?(sledru)
Assignee

Comment 11

a year ago
Sure but, before, I would like to have it running at review phase on all patches which aren't touching thirdparty code.
Flags: needinfo?(sledru)

Comment 12

a year ago
(In reply to Sylvestre Ledru [:sylvestre] from comment #11)
> Sure but, before, I would like to have it running at review phase on all
> patches which aren't touching thirdparty code.

OK. It's up to you. I was just surprised because things relating to the build and/or linting hooks changed (ie I got error/warning messages that made no sense to me ("You don't have this thing you've never heard of and didn't know you needed, here's how to get it!") without warning beforehand. I would imagine I'm not the only person who's surprised...
Yeah, that's fair. In hindsight I should have ensured that there was some kind of automated bootstrapping before granting review, that was my fault.

We filed bug 1434337 and bug 1434332 to codify the rules for adding new linters to the tree a bit. I'm also in the progress of hooking up lint dependency installation to |mach bootstrap|, and I'll add bootstrapping code for shellcheck and codespell while I'm at it.

Updated

a year ago
Product: Testing → Firefox Build System
Assignee

Updated

a year ago
Blocks: 1443508
Assignee

Updated

a year ago
Depends on: 1444048
Assignee

Updated

a year ago
Depends on: 1445245
Assignee

Updated

a year ago
Blocks: 1448934
Assignee

Updated

a year ago
Depends on: 1455086
Assignee

Updated

4 months ago
Depends on: 1523092
Assignee

Updated

5 days ago
Depends on: 1552430
Assignee

Updated

5 days ago
Depends on: 1548485
Assignee

Updated

5 days ago
Depends on: 1552560
Assignee

Updated

a day ago
Alias: codespell
Assignee

Updated

a day ago
Depends on: 1553060
You need to log in before you can comment on or make changes to this bug.