Closed
Bug 1463425
Opened 6 years ago
Closed 6 years ago
flake8/pep8 more code
Categories
(Developer Infrastructure :: Lint and Formatting, defect)
Developer Infrastructure
Lint and Formatting
Tracking
(firefox62 fixed)
RESOLVED
FIXED
mozilla62
Tracking | Status | |
---|---|---|
firefox62 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
Details
Attachments
(6 files)
59 bytes,
text/x-review-board-request
|
ahal
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
gps
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
gps
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
gps
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
gps
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
ahal
:
review+
|
Details |
No description provided.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•6 years ago
|
Attachment #8979592 -
Flags: review?(core-build-config-reviews) → review?(gps)
Attachment #8979593 -
Flags: review?(core-build-config-reviews) → review?(gps)
Attachment #8979594 -
Flags: review?(core-build-config-reviews) → review?(gps)
Attachment #8979595 -
Flags: review?(core-build-config-reviews) → review?(gps)
Comment 7•6 years ago
|
||
mozreview-review |
Comment on attachment 8979592 [details]
bug 1463425 - autopep8 on build/
https://reviewboard.mozilla.org/r/245732/#review251934
::: build/util/count_ctors.py:28
(Diff revision 1)
> - section_name, contents, size, align = f[1], f[2], int(f[5], 16), int(f[10])
> + section_name, contents, size, align = f[1], f[2], int(
> + f[5], 16), int(f[10])
This reformatting made me a little sad. I really wish it would keep the entire expression for the argument on one line. Oh well.
Attachment #8979592 -
Flags: review?(gps) → review+
Comment 8•6 years ago
|
||
mozreview-review |
Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/
https://reviewboard.mozilla.org/r/245734/#review251936
Aside from the possibly justified bareword `except` usage, this looks pretty straightforward.
::: build/mobile/remoteautomation.py:415
(Diff revision 1)
> - except:
> + except Exception:
> pass
> time.sleep(3)
> # Trigger a breakpad dump with "kill -6" (SIGABRT)
> try:
> self.device.pkill(self.procName, sig=6, attempts=1)
> - except:
> + except Exception:
> pass
> # Wait for process to end
> retries = 0
> while retries < 3:
> if self.device.process_exist(self.procName):
> - print "%s still alive after SIGABRT: waiting..." % self.procName
> + print("%s still alive after SIGABRT: waiting..." % self.procName)
> time.sleep(5)
> else:
> return
> retries += 1
> try:
> self.device.pkill(self.procName, sig=9, attempts=1)
> - except:
> - print "%s still alive after SIGKILL!" % self.procName
> + except Exception:
> + print("%s still alive after SIGKILL!" % self.procName)
These *might* be legitimate uses of bareword `except` because it deals with cleaning up the state of a remote device. I don't know the code well enough to say off the top of my head. I'd feel better if whoever maintained this code reviewed these changes. But I can look into it if necessary.
Perhaps we could put a `noqa` on these lines for the time being?
Attachment #8979593 -
Flags: review?(gps) → review-
Comment 9•6 years ago
|
||
mozreview-review |
Comment on attachment 8979594 [details]
bug 1463425 - autopep8 on config/
https://reviewboard.mozilla.org/r/245736/#review251942
Attachment #8979594 -
Flags: review?(gps) → review+
Comment 10•6 years ago
|
||
mozreview-review |
Comment on attachment 8979595 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in config/
https://reviewboard.mozilla.org/r/245738/#review251944
Attachment #8979595 -
Flags: review?(gps) → review+
Comment 11•6 years ago
|
||
mozreview-review |
Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/
https://reviewboard.mozilla.org/r/245730/#review251946
Attachment #8979591 -
Flags: review+
Comment 12•6 years ago
|
||
mozreview-review-reply |
Comment on attachment 8979592 [details]
bug 1463425 - autopep8 on build/
https://reviewboard.mozilla.org/r/245732/#review251934
> This reformatting made me a little sad. I really wish it would keep the entire expression for the argument on one line. Oh well.
Looks like this was generated with max-line-length=80 though our in-tree flake8 configs only error out at 100.
Next time please use `autopep8 --max-line-length=100`. I should really get around to implementing bug 1388721 :)
Comment 13•6 years ago
|
||
mozreview-review |
Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/
https://reviewboard.mozilla.org/r/245730/#review251966
Attachment #8979591 -
Flags: review?(ahal) → review+
Comment 14•6 years ago
|
||
mozreview-review |
Comment on attachment 8979596 [details]
bug 1463425 - Add more directories to the flake8 list
https://reviewboard.mozilla.org/r/245740/#review251968
Thanks for doing this!
Attachment #8979596 -
Flags: review?(ahal) → review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 25•6 years ago
|
||
> This reformatting made me a little sad. I really wish it would keep the
> entire expression for the argument on one line. Oh well.
I fixed it!
Comment 26•6 years ago
|
||
mozreview-review |
Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/
https://reviewboard.mozilla.org/r/245734/#review252270
Attachment #8979593 -
Flags: review?(gps) → review+
Comment 27•6 years ago
|
||
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.
hg error in cmd: hg rebase -s 167126c200248c33a5c23adb345254adf004586a -d 366a9f1b95f6: rebasing 464870:167126c20024 "bug 1463425 - Fix flake8 in browser/ r=ahal,gps"
rebasing 464871:e1f81fce9254 "bug 1463425 - autopep8 on build/ r=gps"
other [source] changed build/win32/procmem.py which local [dest] deleted
use (c)hanged version, leave (d)eleted, or leave (u)nresolved? u
unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 34•6 years ago
|
||
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/02159d1ec622
Fix flake8 in browser/ r=ahal,gps
https://hg.mozilla.org/integration/autoland/rev/c37fb2bf78d6
autopep8 on build/ r=gps
https://hg.mozilla.org/integration/autoland/rev/afa720beeefb
Fix flake8/pep8 issue by hand in build/ r=gps
https://hg.mozilla.org/integration/autoland/rev/fba6f974041a
autopep8 on config/ r=gps
https://hg.mozilla.org/integration/autoland/rev/da3c81f986fa
Fix flake8/pep8 issue by hand in config/ r=gps
https://hg.mozilla.org/integration/autoland/rev/f6f59b7657ac
Add more directories to the flake8 list r=ahal
Comment 35•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/02159d1ec622
https://hg.mozilla.org/mozilla-central/rev/c37fb2bf78d6
https://hg.mozilla.org/mozilla-central/rev/afa720beeefb
https://hg.mozilla.org/mozilla-central/rev/fba6f974041a
https://hg.mozilla.org/mozilla-central/rev/da3c81f986fa
https://hg.mozilla.org/mozilla-central/rev/f6f59b7657ac
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox62:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
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
•