Closed Bug 1463425 Opened 5 years ago Closed 5 years ago

flake8/pep8 more code

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

Product:
Developer Infrastructure
Component:
Lint and Formatting
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox62 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla62
Tracking Flags:
Tracking Status
firefox62 --- fixed

People

(Reporter: Sylvestre, Assigned: Sylvestre)

References

Details

Attachments

(6 files)

bug 1463425 - Fix flake8 in browser/
59 bytes, text/x-review-board-request
ahal
: review+
Details
bug 1463425 - autopep8 on build/
59 bytes, text/x-review-board-request
gps
: review+
Details
bug 1463425 - Fix flake8/pep8 issue by hand in build/
59 bytes, text/x-review-board-request
gps
: review+
Details
bug 1463425 - autopep8 on config/
59 bytes, text/x-review-board-request
gps
: review+
Details
bug 1463425 - Fix flake8/pep8 issue by hand in config/
59 bytes, text/x-review-board-request
gps
: review+
Details
bug 1463425 - Add more directories to the flake8 list
59 bytes, text/x-review-board-request
ahal
: review+
Details 
      No description provided.
Blocks: flake8
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 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 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 on attachment 8979594 [details]
bug 1463425 - autopep8 on config/

https://reviewboard.mozilla.org/r/245736/#review251942
Attachment #8979594 - Flags: review?(gps) → 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 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 on attachment 8979591 [details]
bug 1463425 - Fix flake8 in browser/

https://reviewboard.mozilla.org/r/245730/#review251966
Attachment #8979591 - Flags: review?(ahal) → 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+
> 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 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+
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)
Depends on: 1464866
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.