1232700 - Mozreview python code should follow the pep8 style guide

Status: NEW

Description

[Mauro Doglio [:mdoglio]]

Running flake8 on mozreview raises several errors. Let's run autopep8 on it.

Comment 1

[Mauro Doglio [:mdoglio]]

Created attachment 8698947 [details]
MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28143/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28143/

Comment 2

[Mauro Doglio [:mdoglio]]

Created attachment 8698948 [details]
MozReview Request: mozreview: fix pep8 blank line errors (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28145/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28145/

Comment 3

[Mauro Doglio [:mdoglio]]

Created attachment 8698949 [details]
MozReview Request: mozreview: fix pep8 indentation errors (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28147/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28147/

Comment 4

[Mauro Doglio [:mdoglio]]

Created attachment 8698950 [details]
MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28149/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28149/

Comment 5

[Mauro Doglio [:mdoglio]]

Created attachment 8698951 [details]
MozReview Request: mozreview: fix pep8 error for use of semicolon (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28151/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28151/

Comment 6

[Mauro Doglio [:mdoglio]]

Created attachment 8698952 [details]
MozReview Request: mozreview: fix pep8 error for blank lines at end of file (bug 1232700); r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28153/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28153/

Comment 7

[Mauro Doglio [:mdoglio]]

Created attachment 8698953 [details]
MozReview Request: mozreview: fix flake8 errors (bug 1232700);  r?smacleod

Review commit: https://reviewboard.mozilla.org/r/28155/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28155/

Comment 8

[Mauro Doglio [:mdoglio]]

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 9

[Mauro Doglio [:mdoglio]]

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 10

[Mauro Doglio [:mdoglio]]

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 11

[Mauro Doglio [:mdoglio]]

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 12

[Mauro Doglio [:mdoglio]]

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 13

[Mauro Doglio [:mdoglio]]

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 14

[Mauro Doglio [:mdoglio]]

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/

Comment 15

[Steven MacLeod [: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/#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'[e-mail]', b'mjane', 2001, b'Mary Jane')
> +            self.create_ldap(
> +    b'[e-mail]',
> +    b'mjane',
> +    2001,
> +     b'Mary Jane')

wonky indentation.

::: pylib/mozreview/mozreview/tests/test-autoland-try.py:31
(Diff revision 2)
> -            self.create_ldap(b'[e-mail]', b'mjane', 2001, b'Mary Jane')
> +            self.create_ldap(
> +    b'[e-mail]',
> +    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'[e-mail]', b'mjane', 2001, b'Mary Jane')
> +            self.create_ldap(
> +    b'[e-mail]',
> +    b'mjane',
> +    2001,
> +     b'Mary Jane')

wonky.

Comment 16

[Steven MacLeod [: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

Comment 17

[Steven MacLeod [:smacleod]]

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 (`

Comment 18

[Steven MacLeod [:smacleod]]

Comment on attachment 8698950 [details]
MozReview Request: mozreview: fix pep8 white space errors (bug 1232700); r?smacleod

https://reviewboard.mozilla.org/r/28149/#review26185

Comment 19

[Steven MacLeod [:smacleod]]

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

Comment 20

[Steven MacLeod [:smacleod]]

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.

Comment 21

[Steven MacLeod [: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?

Comment 22

[Mauro Doglio [:mdoglio]]

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 23

[Mauro Doglio [:mdoglio]]

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/

Comment 24

[Mauro Doglio [:mdoglio]]

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 25

[Mauro Doglio [:mdoglio]]

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 26

[Mauro Doglio [:mdoglio]]

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 27

[Mauro Doglio [:mdoglio]]

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 28

[Mauro Doglio [:mdoglio]]

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/

Comment 29

[Mauro Doglio [:mdoglio]]

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/

Comment 30

[Steven MacLeod [:smacleod]]

Comment on attachment 8698953 [details]
MozReview Request: mozreview: fix flake8 errors (bug 1232700);  r?smacleod

https://reviewboard.mozilla.org/r/28155/#review27695

Comment 31

[Steven MacLeod [:smacleod]]

Comment on attachment 8698947 [details]
MozReview Request: mozreview: fix pep8 `line too long` error (bug 1232700); r?smacleod

Clearing flag until issues are fixed.

Comment 32

[Mauro Doglio [:mdoglio]]

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 33

[Mauro Doglio [:mdoglio]]

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/

Comment 34

[Mauro Doglio [:mdoglio]]

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 35

[Mauro Doglio [:mdoglio]]

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 36

[Mauro Doglio [:mdoglio]]

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 37

[Mauro Doglio [:mdoglio]]

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 38

[Mauro Doglio [:mdoglio]]

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 39

[Mauro Doglio [:mdoglio]]

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

[Steven MacLeod [:smacleod]]

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

Comment 41

[Steven MacLeod [: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 42

[Mauro Doglio [:mdoglio]]

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/

Comment 43

[Mauro Doglio [:mdoglio]]

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 44

[Mauro Doglio [:mdoglio]]

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 45

[Mauro Doglio [:mdoglio]]

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 46

[Mauro Doglio [:mdoglio]]

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 47

[Mauro Doglio [:mdoglio]]

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 48

[Mauro Doglio [:mdoglio]]

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 49

[Mauro Doglio [:mdoglio]]

Created attachment 8713139 [details]
MozReview Request: fixup 4851

Review commit: https://reviewboard.mozilla.org/r/32747/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/32747/

Comment 50

[Mauro Doglio [:mdoglio]]

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/

Comment 51

[Mauro Doglio [:mdoglio]]

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 52

[Mauro Doglio [:mdoglio]]

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 53

[Mauro Doglio [:mdoglio]]

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 54

[Mauro Doglio [:mdoglio]]

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 55

[Mauro Doglio [:mdoglio]]

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 56

[Mauro Doglio [:mdoglio]]

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 57

[Mauro Doglio [:mdoglio]]

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 58

[Mauro Doglio [:mdoglio]]

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 59

[Mauro Doglio [:mdoglio]]

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 60

[Mauro Doglio [:mdoglio]]

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 61

[Mauro Doglio [:mdoglio]]

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 62

[Mauro Doglio [:mdoglio]]

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 63

[Mauro Doglio [:mdoglio]]

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 64

[Mauro Doglio [:mdoglio]]

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

[Steven MacLeod [: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/#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.

Comment 66

[Mauro Doglio [:mdoglio]]

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 67

[Mauro Doglio [:mdoglio]]

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 68

[Mauro Doglio [:mdoglio]]

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 69

[Mauro Doglio [:mdoglio]]

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 70

[Mauro Doglio [:mdoglio]]

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 71

[Mauro Doglio [:mdoglio]]

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 72

[Mauro Doglio [:mdoglio]]

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/

Comment 73

[Mauro Doglio [:mdoglio]]

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 74

[Mauro Doglio [:mdoglio]]

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 75

[Mauro Doglio [:mdoglio]]

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 76

[Mauro Doglio [:mdoglio]]

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 77

[Mauro Doglio [:mdoglio]]

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 78

[Mauro Doglio [:mdoglio]]

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 79

[Mauro Doglio [:mdoglio]]

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

[Gregory Szorc [:gps]]

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

[Gregory Szorc [:gps]]

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

Comment 82

[Mauro Doglio [:mdoglio]]

I think this will be a wontfix.