Closed
Bug 1232700
Opened 9 years ago
Closed 6 years ago
Mozreview python code should follow the pep8 style guide
Categories
(MozReview Graveyard :: Review Board: Extension, defect)
MozReview Graveyard
Review Board: Extension
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: mdoglio, Unassigned)
Details
Attachments
(7 files, 1 obsolete file)
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
smacleod
:
review+
|
Details |
Running flake8 on mozreview raises several errors. Let's run autopep8 on it.
Reporter | ||
Comment 1•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28143/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28143/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 2•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28145/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28145/
Attachment #8698948 -
Flags: review?(smacleod)
Reporter | ||
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28147/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28147/
Attachment #8698949 -
Flags: review?(smacleod)
Reporter | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28149/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28149/
Attachment #8698950 -
Flags: review?(smacleod)
Reporter | ||
Comment 5•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28151/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28151/
Attachment #8698951 -
Flags: review?(smacleod)
Reporter | ||
Comment 6•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28153/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28153/
Attachment #8698952 -
Flags: review?(smacleod)
Reporter | ||
Comment 7•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/28155/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/28155/
Attachment #8698953 -
Flags: review?(smacleod)
Reporter | ||
Comment 8•9 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/1-2/
Reporter | ||
Comment 9•9 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/1-2/
Reporter | ||
Comment 10•9 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/1-2/
Reporter | ||
Comment 11•9 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/1-2/
Reporter | ||
Comment 12•9 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/1-2/
Reporter | ||
Comment 13•9 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/1-2/
Reporter | ||
Comment 14•9 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/1-2/
Reporter | ||
Updated•9 years ago
|
Assignee: nobody → mdoglio
Status: NEW → ASSIGNED
Comment 15•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28143/#review26175 ::: pylib/mozreview/mozreview/autoland/resources.py:56 (Diff revision 2) > - return 'autoland_lock:{0}:{1}:{2}'.format(review_request_id, repository_url, > - revision) > + return 'autoland_lock:{0}:{1}:{2}'.format( > + review_request_id, repository_url, revision) indentation is funky here ::: pylib/mozreview/mozreview/autoland/resources.py:103 (Diff revision 2) > + This kicks off Autoland to inbound for a particular review request. I actually think this part of the docstring is uneeded, we should just nuke it. ::: pylib/mozreview/mozreview/autoland/resources.py:613 (Diff revision 2) > - prs = ImportPullRequestRequest.objects.filter(github_user=github_user, > + prs = ImportPullRequestRequest.objects.filter( > + github_user=github_user, > - github_repo=github_repo, > + github_repo=github_repo, > - github_pullrequest=pullrequest) > + github_pullrequest=pullrequest) This indentation is extremely wonky. ::: pylib/mozreview/mozreview/autoland/resources.py:758 (Diff revision 2) > - for field_name in fields: > - assert type(fields[field_name]) == self.fields[field_name]['type'] > + for field in fields: > + assert type(fields[field]) == self.fields[field]['type'] I'd prefer `for name in fields` since it's not the actual field that's returned. ::: pylib/mozreview/mozreview/extension.py:112 (Diff revision 2) > - 'mozreview/js/diffviewer_customizations.js'], > + 'mozreview/js/diffviewer_customizations.js'], It's preferable to put the `],` on its own line and have a trailing comma after the last item in the list here. ::: pylib/mozreview/mozreview/extension.py:159 (Diff revision 2) > - HeaderDropdownActionHook(self, actions=[{ > + HeaderDropdownActionHook(self, actions=[ > + { > - 'label': 'MozReview', > + 'label': 'MozReview', > - 'items': [ > + 'items': [ > - { > + { > - 'label': 'User Guide', > + 'label': 'User Guide', > - 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview-user.html', > + 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/mozreview-user.html', # noqa > - }, > + }, > - { > + { > - 'label': 'Mercurial for Mozillians', > + 'label': 'Mercurial for Mozillians', > - 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/index.html', > + 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/hgmozilla/index.html', # noqa > - }, > + }, > - { > + { > - 'label': 'Hacking MozReview', > + 'label': 'Hacking MozReview', > - 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/hacking-mozreview.html', > + 'url': 'https://mozilla-version-control-tools.readthedocs.org/en/latest/hacking-mozreview.html', # noqa > - }, > + }, > - { > + { > - 'label': 'File a Bug', > + 'label': 'File a Bug', > - 'url': 'https://bugzilla.mozilla.org/enter_bug.cgi?product=Developer%20Services&component=MozReview', > + 'url': 'https://bugzilla.mozilla.org/enter_bug.cgi?product=Developer%20Services&component=MozReview', # noqa > - }, > + }, > - ], > + ], I'd revert the change here which drops the `{` to its own line and forces an indent of everything. For the long strings I would take advantage of python's multiline implicit string continuation and do something like this: { 'label': 'User Guide', 'url': 'https://mozilla-version-control-tools.' 'readthedocs.org/en/latest/mozreview-user.html', }, ::: pylib/mozreview/mozreview/extension.py:272 (Diff revision 2) > - url(r'^import-pullrequest/(?P<user>.+)/(?P<repo>.+)/(?P<pullrequest>\d+)/$', > - import_pullrequest, name='import_pullrequest'))) > + r'^import-pullrequest/(?P<user>.+)/(?P<repo>.+)/(?P<pullrequest>\d+)/$', # noqa > + import_pullrequest, > + name='import_pullrequest') this indentation is quite woky, we've got arguments to `url` at differnt levels. (Also avoiding noqa with continuations is preferable IMO) ::: pylib/mozreview/mozreview/extra_data.py:135 (Diff revision 2) > - parent_rr_draft.extra_data[REVIEWER_MAP_KEY] = json.dumps(reviewers_map_after) > + parent_rr_draft.extra_data[ > + REVIEWER_MAP_KEY] = json.dumps(reviewers_map_after) I'd much rather keep the dictionary index part intact and drop json.dumps to a new line using parens: parent_rr_draft.extra_data[REVIEWER_MAP_KEY] = ( json.dumps(reviewers_map_after)) or alternatively if it fits: parent_rr_draft.extra_data[REVIEWER_MAP_KEY] = json.dumps( reviewers_map_after) ::: pylib/mozreview/mozreview/fields.py:74 (Diff revision 2) > - parent = get_parent_rr(self.review_request_details.get_review_request()) > + parent = get_parent_rr( > + self.review_request_details.get_review_request()) This is pretty wonky here. ::: pylib/mozreview/mozreview/fields.py:146 (Diff revision 2) > - parent = get_parent_rr(self.review_request_details.get_review_request()) > + parent = get_parent_rr( > + self.review_request_details.get_review_request()) again, the dropping indentation level to below the block is really strange. ::: pylib/mozreview/mozreview/fields.py:183 (Diff revision 2) > return (is_pushed(self.review_request_details) and > is_parent(self.review_request_details) and > - False) # TODO: Remove and render hg web link to the base commit. > + False) > + # TODO: Remove and render hg web link to the base commit. This seems quite silly that we're executing these functions just to always return `False`. Please just remove the function calls and put this comment above the `return` ::: pylib/mozreview/mozreview/resources/review_request_summary.py:204 (Diff revision 2) > - 'href': 'http://127.0.0.1:50936/api/extensions/mozreview.extension.MozReviewExtension/summary/3/', > + 'href': 'http://127.0.0.1:50936/api/extensions/mozreview.extension.MozReviewExtension/summary/3/', # noqa continuation please ::: pylib/mozreview/mozreview/tests/test-autoland-concurrent.py:69 (Diff revision 2) > - self.create_ldap(b'mjane@example.com', b'mjane', 2001, b'Mary Jane') > + self.create_ldap( > + b'mjane@example.com', > + b'mjane', > + 2001, > + b'Mary Jane') wonky indentation. ::: pylib/mozreview/mozreview/tests/test-autoland-try.py:31 (Diff revision 2) > - self.create_ldap(b'mjane@example.com', b'mjane', 2001, b'Mary Jane') > + self.create_ldap( > + b'mjane@example.com', > + b'mjane', > + 2001, > + b'Mary Jane') Wonky indentation. ::: pylib/mozreview/mozreview/tests/test-browser-cache.py:87 (Diff revision 2) > - WebDriverWait(self.browser, 3).until( > - EC.visibility_of_element_located((By.XPATH, "id('issue-summary')"))) > + WebDriverWait( > + self.browser, 3).until( > + EC.visibility_of_element_located( > + (By.XPATH, "id('issue-summary')"))) wonky. ::: pylib/mozreview/mozreview/tests/test-import-pullrequest.py:28 (Diff revision 2) > - self.create_ldap(b'mjane@example.com', b'mjane', 2001, b'Mary Jane') > + self.create_ldap( > + b'mjane@example.com', > + b'mjane', > + 2001, > + b'Mary Jane') wonky.
Attachment #8698947 -
Flags: review?(smacleod)
Comment 16•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28145/#review26181
Attachment #8698948 -
Flags: review?(smacleod) → review+
Comment 17•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28147/#review26183 ::: pylib/mozreview/mozreview/autoland/resources.py:334 (Diff revision 2) > - response = requests.post(autoland_url + '/autoland', > + response = requests.post( > + autoland_url + '/autoland', > data=json.dumps({ > - 'ldap_username': request.mozreview_profile.ldap_username, > + 'ldap_username': request.mozreview_profile.ldap_username, > - 'tree': rr.repository.name, > + 'tree': rr.repository.name, > - 'pingback_url': pingback_url, > + 'pingback_url': pingback_url, > - 'rev': last_revision, > + 'rev': last_revision, > - 'destination': TRY_AUTOLAND_DESTINATION, > + 'destination': TRY_AUTOLAND_DESTINATION, > - 'trysyntax': try_syntax, > + 'trysyntax': try_syntax, > - }), headers={ > - 'content-type': 'application/json', > + }), > + headers={'content-type': 'application/json'}, > - }, didn't this change show up in a previous commit?? ::: pylib/mozreview/mozreview/middleware.py:29 (Diff revision 2) > if (request.resolver_match and > - request.resolver_match.url_name in self.URLNAME_BLACKLIST and > + request.resolver_match.url_name in self.URLNAME_BLACKLIST and > - 'HTTP_IF_NONE_MATCH' in request.META): > + 'HTTP_IF_NONE_MATCH' in request.META): These situations make me hate the length of `if (`
Attachment #8698949 -
Flags: review?(smacleod) → review+
Comment 18•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28149/#review26185
Attachment #8698950 -
Flags: review?(smacleod) → review+
Comment 19•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28151/#review26187
Attachment #8698951 -
Flags: review?(smacleod) → review+
Comment 20•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28153/#review26189 Your commit summary line is misleading, please update. ::: pylib/mozreview/mozreview/autoland/views.py (Diff revision 2) > - dummy comment to open issue to make sure you see the general review comment.
Attachment #8698952 -
Flags: review?(smacleod) → review+
Updated•8 years ago
|
Attachment #8698953 -
Flags: review?(smacleod)
Comment 21•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28155/#review26191 ::: pylib/mozreview/mozreview/autoland/resources.py (Diff revision 2) > - 'request_id': autoland_request_id, why did you nuke the request id here?
Reporter | ||
Comment 22•8 years ago
|
||
https://reviewboard.mozilla.org/r/28155/#review26191 > why did you nuke the request id here? It's an error, `autoland_request_id` is undefined. And if we used `request_id`, that wouldn't be defined either in case of an exception.
Reporter | ||
Comment 23•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/2-3/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 24•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/2-3/
Reporter | ||
Comment 25•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/2-3/
Reporter | ||
Comment 26•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/2-3/
Reporter | ||
Comment 27•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/2-3/
Reporter | ||
Comment 28•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/2-3/
Attachment #8698952 -
Attachment description: MozReview Request: mozreview: fix pep8 error for blank lines (bug 1232700); r?smacleod → MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod
Reporter | ||
Comment 29•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/2-3/
Attachment #8698953 -
Flags: review?(smacleod)
Comment 30•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28155/#review27695
Attachment #8698953 -
Flags: review?(smacleod) → review+
Comment 31•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Clearing flag until issues are fixed.
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 32•8 years ago
|
||
https://reviewboard.mozilla.org/r/28143/#review26175 > This indentation is extremely wonky. This is an issue with the diff viewer. These lines have all the same indentation in my editor, like ```python prs = ImportPullRequestRequest.objects.filter( github_user=github_user, github_repo=github_repo, github_pullrequest=pullrequest) ```
Reporter | ||
Comment 33•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/3-4/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 34•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/3-4/
Reporter | ||
Comment 35•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/3-4/
Reporter | ||
Comment 36•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/3-4/
Reporter | ||
Comment 37•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/3-4/
Reporter | ||
Comment 38•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/3-4/
Reporter | ||
Comment 39•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/3-4/
Comment 40•8 years ago
|
||
https://reviewboard.mozilla.org/r/28143/#review26175 > This is an issue with the diff viewer. These lines have all the same indentation in my editor, like > ```python > prs = ImportPullRequestRequest.objects.filter( > github_user=github_user, > github_repo=github_repo, > github_pullrequest=pullrequest) > ``` This is definitely a problem in the commit: see https://reviewboard-hg.mozilla.org/version-control-tools/rev/8dd9cfac64f7
Updated•8 years ago
|
Attachment #8698947 -
Flags: review?(smacleod)
Comment 41•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28143/#review28011 There are still a number of issues open - clearing flag until issues are resolved.
Reporter | ||
Comment 42•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/4-5/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 43•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/4-5/
Reporter | ||
Comment 44•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/4-5/
Reporter | ||
Comment 45•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/4-5/
Reporter | ||
Comment 46•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/4-5/
Reporter | ||
Comment 47•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/4-5/
Reporter | ||
Comment 48•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/4-5/
Reporter | ||
Comment 49•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/32747/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/32747/
Reporter | ||
Comment 50•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/5-6/
Reporter | ||
Updated•8 years ago
|
Attachment #8713139 -
Attachment is obsolete: true
Reporter | ||
Comment 51•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/5-6/
Reporter | ||
Comment 52•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/5-6/
Reporter | ||
Comment 53•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/5-6/
Reporter | ||
Comment 54•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/5-6/
Reporter | ||
Comment 55•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/5-6/
Reporter | ||
Comment 56•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/5-6/
Reporter | ||
Comment 57•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/6-7/
Reporter | ||
Comment 58•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/6-7/
Reporter | ||
Comment 59•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/6-7/
Reporter | ||
Comment 60•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/6-7/
Reporter | ||
Comment 61•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/6-7/
Reporter | ||
Comment 62•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/6-7/
Reporter | ||
Comment 63•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/6-7/
Reporter | ||
Comment 64•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/7-8/
Comment 65•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod https://reviewboard.mozilla.org/r/28143/#review30649 Looks fine to me for the most part - you're going to have a bunch of merge conflicts when you rebase this on top of the CommitData.extra_data work though. In those cases you should probably just take my changes since I made sure to not introduce anymore flake8 errors and removed a bunch as well.
Attachment #8698947 -
Flags: review?(smacleod) → review+
Reporter | ||
Comment 66•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/7-8/
Reporter | ||
Comment 67•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/7-8/
Reporter | ||
Comment 68•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/7-8/
Reporter | ||
Comment 69•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/7-8/
Reporter | ||
Comment 70•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/7-8/
Reporter | ||
Comment 71•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/7-8/
Reporter | ||
Comment 72•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/8-9/
Attachment #8698953 -
Attachment description: MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod → MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod
Reporter | ||
Comment 73•8 years ago
|
||
Comment on attachment 8698947 [details] MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28143/diff/8-9/
Reporter | ||
Comment 74•8 years ago
|
||
Comment on attachment 8698948 [details] MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28145/diff/8-9/
Reporter | ||
Comment 75•8 years ago
|
||
Comment on attachment 8698949 [details] MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28147/diff/8-9/
Reporter | ||
Comment 76•8 years ago
|
||
Comment on attachment 8698950 [details] MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28149/diff/8-9/
Reporter | ||
Comment 77•8 years ago
|
||
Comment on attachment 8698951 [details] MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28151/diff/8-9/
Reporter | ||
Comment 78•8 years ago
|
||
Comment on attachment 8698952 [details] MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28153/diff/8-9/
Reporter | ||
Comment 79•8 years ago
|
||
Comment on attachment 8698953 [details] MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod Review request updated; see interdiff: https://reviewboard.mozilla.org/r/28155/diff/9-10/
Comment 80•8 years ago
|
||
I'm backing this all out because of multiple exceptions in production. Some appear to resemble merge issues. I highly suggest submitting the final/rebased commits to MozReview before landing again.
Comment 81•8 years ago
|
||
You'll want to graft the following changesets to fixup the changesets that landed: https://hg.mozilla.org/hgcustom/version-control-tools/rev/a6cae387f33d https://hg.mozilla.org/hgcustom/version-control-tools/rev/f4f1771bd042
Assignee | ||
Updated•8 years ago
|
Product: Developer Services → MozReview
Updated•8 years ago
|
Component: General → Review Board: Extension
Reporter | ||
Updated•8 years ago
|
Status: ASSIGNED → NEW
Reporter | ||
Comment 82•8 years ago
|
||
I think this will be a wontfix.
Reporter | ||
Updated•8 years ago
|
Assignee: mdoglio → nobody
Comment 83•6 years ago
|
||
MozReview is now obsolete. Please use Phabricator instead. Closing this bug.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•