Closed
Bug 1449035
Opened 6 years ago
Closed 6 years ago
Codespell: silent warnings about binary errors
Categories
(Developer Infrastructure :: Source Code Analysis, enhancement, P5)
Developer Infrastructure
Source Code Analysis
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Sylvestre)
Details
Attachments
(1 file)
+++ This bug was initially created as a clone of Bug #1444048 +++ To silent: Unable to match regex against output: WARNING: Binary file: /home/sylvestre/dev/mozilla/mozilla-central.hg/tools/lint/python/compat.pyc Moving to --quiet-level=7 silent them.
Comment hidden (mozreview-request) |
Comment 2•6 years ago
|
||
mozreview-review |
Comment on attachment 8962533 [details] Bug 1449035 - Codespell: move to quiet-level=7 to also ignore the binary https://reviewboard.mozilla.org/r/231328/#review237078 I think there's a different issue here: https://searchfox.org/mozilla-central/rev/49cc27555d5b7ed2eb06baf156693dd578daa06f/tools/lint/codespell.yml#23 pyc isn't listed, so if it is being matched, there's a bug in the lint code. Generally I think we need to make sure we exclude (as we don't need to be testing these files anyway), rather than turn down the warnings.
Attachment #8962533 -
Flags: review?(standard8)
Comment 3•6 years ago
|
||
mozreview-review |
Comment on attachment 8962533 [details] Bug 1449035 - Codespell: move to quiet-level=7 to also ignore the binary https://reviewboard.mozilla.org/r/231328/#review237084
Attachment #8962533 -
Flags: review-
Assignee | ||
Comment 4•6 years ago
|
||
Mark, in the initial implementation, I thought that the list of extensions was managed by mozlint itself and not by the checkers themselves. Now, I know that it isn't the case. That means that the list https://searchfox.org/mozilla-central/rev/49cc27555d5b7ed2eb06baf156693dd578daa06f/tools/lint/codespell.yml#23 is unused. Now, codespell doesn't give an option to limit the extensions being checked. Only to ignore a few. I see a few options here: 1) We add quiet-level=7 and remove the extensions list to avoid 2) Maintain a list like --skip=*.dic,*.png,*.pyc,*.bmp,*.jpg,*.ico,*.icns,dsstore 3) Implement the feature in codespell upstream
Flags: needinfo?(standard8)
Assignee | ||
Comment 5•6 years ago
|
||
Press "Save changes" too quickly. Please let me know what you think! This is showing a lot of useless warnings in the CI: https://treeherder.mozilla.org/logviewer.html#?job_id=178014892&repo=autoland&lineNumber=247
Flags: needinfo?(ahalberstadt)
Comment 6•6 years ago
|
||
Correct, if you pass in a directory, mozlint will just forward that directory to the underlying linter. This is mainly because otherwise mozlint would have to convert that directory into a list of files and pass the entire thing in (which will exceed the maximum command length on most operating systems if linting the entire tree). If you pass in files or globs, then mozlint *will* use that extension list to pre-filter. Of those 3 options, I think I'd prefer 1 or 3. There is another option though. We could convert directories into lists of files and do our own filtering. There are utilities in-tree to facilitate this, and the py2/py3 linters actually already do this: https://searchfox.org/mozilla-central/source/tools/lint/python/compat.py#71 Once we have our own pre-filtered list of files, we can either pass them into codespell explicitly (being careful not to exceed the maximum command line length), or instead of calling codespell via a subprocess, we could call into its internals directly (as it is written in python).
Flags: needinfo?(ahalberstadt)
Assignee | ||
Comment 7•6 years ago
|
||
For now, I propose that we do 1) for now and 3) later.
Assignee | ||
Updated•6 years ago
|
Assignee: nobody → sledru
Assignee | ||
Updated•6 years ago
|
Summary: Codespell: ignore more stuff → Codespell: silent warnings about binary errors
Comment hidden (mozreview-request) |
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8962533 [details] Bug 1449035 - Codespell: move to quiet-level=7 to also ignore the binary https://reviewboard.mozilla.org/r/231328/#review249686
Attachment #8962533 -
Flags: review?(ahalberstadt) → review+
Comment 10•6 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/ca08a480132a Codespell: move to quiet-level=7 to also ignore the binary r=ahal
Comment 11•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ca08a480132a
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•6 years ago
|
Flags: needinfo?(standard8)
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•