Closed
Bug 1406650
Opened 7 years ago
Closed 7 years ago
Make build/*.py flake8 compatible and add them to the list of files to check
Categories
(Developer Infrastructure :: Source Code Analysis, enhancement)
Developer Infrastructure
Source Code Analysis
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
Details
Attachments
(1 file)
No description provided.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8916260 [details] Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check https://reviewboard.mozilla.org/r/187484/#review193580 This looks good to me, but a build peer will need to grant final review here. Try mshal or chmanchester. ::: commit-message-6b049:1 (Diff revision 2) > +hBug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check r?ahal extra 'h'. This might fail the server side commit hook. ::: tools/lint/flake8.yml:6 (Diff revision 2) > --- > flake8: > description: Python linter > include: > + - configure.py > + - build/*.py This will only lint files that are direct children of `/build`. Was that intentional? If you wanted to lint all files under there, this should be changed to just `build`. I suspect doing this will result in a lot more failures.
Attachment #8916260 -
Flags: review?(ahalberstadt)
Assignee | ||
Comment 4•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8916260 [details] Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check https://reviewboard.mozilla.org/r/187484/#review193580 thanks > extra 'h'. This might fail the server side commit hook. Oups, thanks > This will only lint files that are direct children of `/build`. Was that intentional? If you wanted to lint all files under there, this should be changed to just `build`. I suspect doing this will result in a lot more failures. Yes, this is the goal for now. There are subdirectories not consistent to pep8
Comment hidden (mozreview-request) |
Comment 6•7 years ago
|
||
Comment on attachment 8916260 [details] Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check Thanks for the clarification.
Attachment #8916260 -
Flags: review?(ahalberstadt) → review+
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sledru
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8916260 [details] Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check https://reviewboard.mozilla.org/r/187484/#review193774 ::: build/appini_header.py:30 (Diff revision 3) > - appdata = dict(("%s:%s" % (s, o), config.get(s, o)) for s in config.sections() for o in config.options(s)) > + pass > + appdata = dict(("%s:%s" % (s, o), config.get(s, o)) > + for s in config.sections() for o in config.options(s)) > appdata['flags'] = ' | '.join(flags) if flags else '0' > - appdata['App:profile'] = '"%s"' % appdata['App:profile'] if 'App:profile' in appdata else 'NULL' > + appdata['App:profile'] = ('"%s"' % > + appdata['App:profile'] if 'App:profile' in appdata else 'NULL') Can we break this line before the `if` instead? ::: build/gen_test_packages_manifest.py:47 (Diff revision 3) > - help="Name of the \"common\" archive, a package to be used by all harnesses.") > + help="Name of the \"common\" archive, a package to be used by all " > + "harnesses.") Is flake8 ok with "harnesses" directly below "Name" in this and other cases like it? (At least as mozreview is showing this to me, "harnesses" is directly below `help`). ::: build/upload.py:219 (Diff revision 3) > - properties = {prop: 'UNKNOWN' for prop, condition in property_conditions} > + properties = {prop: 'UNKNOWN' for prop, > + condition in property_conditions} Can we break this line before `in` instead?
Attachment #8916260 -
Flags: review?(cmanchester) → review+
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8916260 [details] Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check https://reviewboard.mozilla.org/r/187484/#review193774 > Is flake8 ok with "harnesses" directly below "Name" in this and other cases like it? (At least as mozreview is showing this to me, "harnesses" is directly below `help`). yeah, flake8 is happy with that
Comment hidden (mozreview-request) |
Comment 10•7 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 48dcf372b45a -d 4b4228043dd8: rebasing 426005:48dcf372b45a "Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check r=chmanchester" (tip) merging tools/lint/flake8.yml warning: conflicts while merging tools/lint/flake8.yml! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment 11•7 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 48dcf372b45a -d 44d7a43c481a: rebasing 426049:48dcf372b45a "Bug 1406650 - Make build/*.py and a few other files flake8 compatible and add them to the list of files to check r=chmanchester" (tip) merging build/build-clang/build-clang.py merging tools/lint/flake8.yml warning: conflicts while merging tools/lint/flake8.yml! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•7 years ago
|
||
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6074db12d685 Make build/*.py and a few other files flake8 compatible and add them to the list of files to check r=chmanchester
Updated•7 years ago
|
Attachment #8916260 -
Flags: review?(ahalberstadt)
Comment 15•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6074db12d685
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Updated•6 years ago
|
Product: Core → Firefox Build System
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
•