Closed Bug 1231810 Opened 8 years ago Closed 7 years ago

testing/*cppun*.py code should follow the flake8 convention

Categories

(Testing :: General, defect)

defect
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: Sylvestre, Assigned: simon)

References

Details

Attachments

(2 files, 1 obsolete file)

Run on the Firefox source code, the following line shows a bunch of warnings. Nothing critical but it would be nice to remove them:
flake8 --max-line-length=4000 testing/*cppun*.py
Blocks: flake8
No longer blocks: 1231806
A while ago ahal ran autopep8 on some of our test harness code. I'd be fine with someone doing that on a larger scale.
See bug 1199939, I started a project with Samy to integrate that better. The checks should be integrated in Mozreview and in our automation at some point.
See also bug 1230962. I have a working prototype with eslint support, pyflakes is the next one I want to add. The idea is that mozreview, ci and a mach command will be consumers of mozlint. I.e add a linter once, and get it running everywhere* for free.

*except maybe editors
Could I take this bug?
Sure! It's currently unassigned, so now it's yours. :)
Assignee: nobody → simon
flake8 was reporting the following issues:
* expected two blank lines
* unexpected spaces around keyword / parameter equals
* wrongly imported modules or multiple imports on one line
* under-indented lines for visual indent
* unidentified name because of a bad call

Review commit: https://reviewboard.mozilla.org/r/30237/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/30237/
So as can be seen above in Comment #6, I submitted a commit via MozReview. I tried to assign it to you by typing r?ted.mielczarek, but it didn't work so you are not the assigned reviewer. Bug filed at https://bugzilla.mozilla.org/show_bug.cgi?id=1238353

anyhow, from that I tried to add you as a reviewer on https://reviewboard.mozilla.org/r/30237/ but I couldn't find how to change it. Bug too?
Attachment #8706088 - Flags: review?(ted)
Comment on attachment 8706088 [details]
MozReview Request: Bug 1231810 - Fix files not following flake8 convention ; r?ted

https://reviewboard.mozilla.org/r/30237/#review27309

Thanks for the patch! Just one minor nit.

::: testing/remotecppunittests.py:19
(Diff revision 1)
> +from mozdevice import devicemanagerSUT

Maybe change this to:
```
from mozdevice import (
    devicemanagerADB,
    devicemanagerSUT,
)
```
Attachment #8706088 - Flags: review?(ted) → review+
Comment on attachment 8706088 [details]
MozReview Request: Bug 1231810 - Fix files not following flake8 convention ; r?ted

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30237/diff/1-2/
Attachment #8706088 - Attachment description: MozReview Request: Bug 1231810 - Fix files not following flake8 convention ; r?ted.mielczarek → MozReview Request: Bug 1231810 - Fix files not following flake8 convention ; r?ted
Attachment #8865667 - Attachment is obsolete: true
Attachment #8865667 - Flags: review?(ted)
ted, I rebased the patch for Simon.
Comment on attachment 8865676 [details]
Bug 1231810 - Fix files not following flake8 convention

https://reviewboard.mozilla.org/r/137300/#review142008

Thanks, sorry we never landed this!
Attachment #8865676 - Flags: review?(ted) → review+
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/20e72e5cc536
Fix files not following flake8 convention r=ted
https://hg.mozilla.org/mozilla-central/rev/20e72e5cc536
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Blocks: 1406666
You need to log in before you can comment on or make changes to this bug.