Closed
Bug 1232700
Opened 10 years ago
Closed 7 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 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•10 years ago
|
Assignee: nobody → mdoglio
Status: NEW → ASSIGNED
Comment 15•10 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•10 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•10 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•10 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•10 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•10 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•10 years ago
|
Attachment #8698953 -
Flags: review?(smacleod)
Comment 21•10 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•9 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•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/2-3/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 24•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/2-3/
Reporter | ||
Comment 25•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/2-3/
Reporter | ||
Comment 26•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/2-3/
Reporter | ||
Comment 27•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/2-3/
Reporter | ||
Comment 28•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/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•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/2-3/
Attachment #8698953 -
Flags: review?(smacleod)
Comment 30•9 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•9 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•9 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•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/3-4/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 34•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/3-4/
Reporter | ||
Comment 35•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/3-4/
Reporter | ||
Comment 36•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/3-4/
Reporter | ||
Comment 37•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/3-4/
Reporter | ||
Comment 38•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/3-4/
Reporter | ||
Comment 39•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/3-4/
Comment 40•9 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•9 years ago
|
Attachment #8698947 -
Flags: review?(smacleod)
Comment 41•9 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•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/4-5/
Attachment #8698947 -
Flags: review?(smacleod)
Reporter | ||
Comment 43•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/4-5/
Reporter | ||
Comment 44•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/4-5/
Reporter | ||
Comment 45•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/4-5/
Reporter | ||
Comment 46•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/4-5/
Reporter | ||
Comment 47•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/4-5/
Reporter | ||
Comment 48•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/4-5/
Reporter | ||
Comment 49•9 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•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/5-6/
Reporter | ||
Updated•9 years ago
|
Attachment #8713139 -
Attachment is obsolete: true
Reporter | ||
Comment 51•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/5-6/
Reporter | ||
Comment 52•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/5-6/
Reporter | ||
Comment 53•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/5-6/
Reporter | ||
Comment 54•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/5-6/
Reporter | ||
Comment 55•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/5-6/
Reporter | ||
Comment 56•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/5-6/
Reporter | ||
Comment 57•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/6-7/
Reporter | ||
Comment 58•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/6-7/
Reporter | ||
Comment 59•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/6-7/
Reporter | ||
Comment 60•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/6-7/
Reporter | ||
Comment 61•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/6-7/
Reporter | ||
Comment 62•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/6-7/
Reporter | ||
Comment 63•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/6-7/
Reporter | ||
Comment 64•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/7-8/
Comment 65•9 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•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/7-8/
Reporter | ||
Comment 67•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/7-8/
Reporter | ||
Comment 68•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/7-8/
Reporter | ||
Comment 69•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/7-8/
Reporter | ||
Comment 70•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/7-8/
Reporter | ||
Comment 71•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/7-8/
Reporter | ||
Comment 72•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/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•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/8-9/
Reporter | ||
Comment 74•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/8-9/
Reporter | ||
Comment 75•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/8-9/
Reporter | ||
Comment 76•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/8-9/
Reporter | ||
Comment 77•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/8-9/
Reporter | ||
Comment 78•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/8-9/
Reporter | ||
Comment 79•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/9-10/
Comment 80•9 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•9 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•9 years ago
|
Product: Developer Services → MozReview
Updated•9 years ago
|
Component: General → Review Board: Extension
Reporter | ||
Updated•9 years ago
|
Status: ASSIGNED → NEW
Reporter | ||
Comment 82•9 years ago
|
||
I think this will be a wontfix.
Reporter | ||
Updated•9 years ago
|
Assignee: mdoglio → nobody
Comment 83•7 years ago
|
||
MozReview is now obsolete. Please use Phabricator instead. Closing this bug.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•