1463425 - flake8/pep8 more code

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - Fix flake8 in browser/

Review commit: https://reviewboard.mozilla.org/r/245730/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245730/

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - autopep8 on build/

Review commit: https://reviewboard.mozilla.org/r/245732/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245732/

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - Fix flake8/pep8 issue by hand in build/

Review commit: https://reviewboard.mozilla.org/r/245734/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245734/

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - autopep8 on config/

Review commit: https://reviewboard.mozilla.org/r/245736/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245736/

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - Fix flake8/pep8 issue by hand in config/

Review commit: https://reviewboard.mozilla.org/r/245738/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245738/

Comment 6

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug 1463425 - Add more directories to the flake8 list

Review commit: https://reviewboard.mozilla.org/r/245740/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245740/

Comment 7

[Gregory Szorc [:gps]]

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.

Comment 8

[Gregory Szorc [:gps]]

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?

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 8979594 [details]
bug 1463425 - autopep8 on config/

https://reviewboard.mozilla.org/r/245736/#review251942

Comment 10

[Gregory Szorc [:gps]]

Comment on attachment 8979595 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in config/

https://reviewboard.mozilla.org/r/245738/#review251944

Comment 11

[Gregory Szorc [:gps]]

Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/

https://reviewboard.mozilla.org/r/245730/#review251946

Comment 12

[Andrew Halberstadt [:ahal]]

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

[Andrew Halberstadt [:ahal]]

Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/

https://reviewboard.mozilla.org/r/245730/#review251966

Comment 14

[Andrew Halberstadt [:ahal]]

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!

Comment 15

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245730/diff/1-2/

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979592 [details]
bug 1463425 - autopep8 on build/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245732/diff/1-2/

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245734/diff/1-2/

Comment 18

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979594 [details]
bug 1463425 - autopep8 on config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245736/diff/1-2/

Comment 19

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979595 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245738/diff/1-2/

Comment 20

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979596 [details]
bug 1463425 - Add more directories to the flake8 list

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245740/diff/1-2/

Comment 21

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245734/diff/2-3/

Comment 22

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979594 [details]
bug 1463425 - autopep8 on config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245736/diff/2-3/

Comment 23

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979595 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245738/diff/2-3/

Comment 24

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979596 [details]
bug 1463425 - Add more directories to the flake8 list

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245740/diff/2-3/

Comment 25

[Sylvestre Ledru [:Sylvestre]]

> 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

[Gregory Szorc [:gps]]

Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/

https://reviewboard.mozilla.org/r/245734/#review252270

Comment 27

[Mozilla Autoland]

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 28

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245730/diff/2-3/

Comment 29

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979592 [details]
bug 1463425 - autopep8 on build/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245732/diff/2-3/

Comment 30

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979593 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in build/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245734/diff/3-4/

Comment 31

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979594 [details]
bug 1463425 - autopep8 on config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245736/diff/3-4/

Comment 32

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979595 [details]
bug 1463425 - Fix flake8/pep8 issue by hand in config/

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245738/diff/3-4/

Comment 33

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8979596 [details]
bug 1463425 - Add more directories to the flake8 list

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/245740/diff/3-4/

Comment 34

[Pulsebot]

Pushed by [e-mail]:
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

[Eliza Balazs [:ebalazs_]]

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