Closed Bug 1232700 Opened 7 years ago Closed 4 years ago

Mozreview python code should follow the pep8 style guide

Categories

(MozReview Graveyard :: Review Board: Extension, defect)

Product:
MozReview Graveyard
Component:
Review Board: Extension
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INVALID

People

(Reporter: mdoglio, Unassigned)

Details

Attachments

(7 files, 1 obsolete file)

MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details
MozReview Request: mozreview: fix flake8 errors (bug 1232700); r?smacleod
58 bytes, text/x-review-board-request
smacleod
: review+
Details 
Running flake8 on mozreview raises several errors. Let's run autopep8 on it.
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/
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/
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/
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/
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/
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/
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/
Assignee: nobody → mdoglio
Status: NEW → ASSIGNED
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 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 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 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 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 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+
Attachment #8698953 - Flags: review?(smacleod)
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?
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.
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)
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/
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/
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/
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/
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
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 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 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)
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)
```
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)
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/
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/
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/
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/
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/
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/
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
Attachment #8698947 - Flags: review?(smacleod)
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.
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)
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/
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/
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/
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/
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/
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/
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/
Attachment #8713139 - Attachment is obsolete: true
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/
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/
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/
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/
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/
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/
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/
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/
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/
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/
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/
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/
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/
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 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+
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/
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/
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/
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/
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/
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/
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
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/
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/
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/
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/
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/
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/
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/
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.
Product: Developer Services → MozReview
Component: General → Review Board: Extension
Status: ASSIGNED → NEW
I think this will be a wontfix.
Assignee: mdoglio → nobody
MozReview is now obsolete. Please use Phabricator instead. Closing this bug.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.