Closed Bug 1433410 (codespell) Opened 6 years ago Closed 6 years ago

Add codespell support for mach lint

Categories

(Developer Infrastructure :: Lint and Formatting, enhancement)

enhancement
Not set
normal

Tracking

(firefox60 fixed)

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Blocks: 1433417
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+
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e414096f1c3b
Add codespell support for mach lint r=ahal
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3eefd4e1087c
Add codespell support for mach lint r=ahal
https://hg.mozilla.org/mozilla-central/rev/3eefd4e1087c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
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)
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)
(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.
Product: Testing → Firefox Build System
Blocks: 1443508
Depends on: 1444048
Depends on: 1445245
Blocks: 1448934
Depends on: 1455086
Depends on: 1523092
Depends on: 1552430
Depends on: 1548485
Depends on: 1552560
Alias: codespell
Depends on: 1553060
Depends on: 1679058
Depends on: 1679053
Depends on: 1717447
Product: Firefox Build System → Developer Infrastructure
Depends on: 1785451
Depends on: 1824421
Depends on: 1840227
Depends on: 1860157
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: