• 
      

    Create a new Review Request after creating a PR on Github

    Review Request #8741 — Created Feb. 12, 2017 and discarded

    Information

    Review Board
    dvcs

    Reviewers

    I didn't have permission to modify r/8437/, so I'm posting this as a separate RR. A decent portion of this diff is stuff that Dominic did. Anything involving social authentication is stuff I did.

    Users may now authenticate with GitHub, associating their RR account with their GitHub account. When a webhook pull request is incoming, it will attempt to find a user with that GitHub username and submit it under their name, else it submits it under a dummy account (No_Submitter).

    I have also added the ability to authenticate with a BitBucket account. The data is currently not showing up in the database even though it successfully authenticates.

    Nothing except testing that it actually works - no unit tests or attempting to break it.

    Description From Last Updated

    There's some weird stuff going on in settings.py. It looks like you may have squashed some changes into this one.

    brennie brennie

    Col: 5 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 13 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 9 E265 block comment should start with '# '

    reviewbot reviewbot

    We should use social-auth-core and social-auth-app-django for this. See http://python-social-auth.readthedocs.io/en/latest/. We don't want to limit this too just GitHub. Ideally …

    brennie brennie

    Col: 1 E302 expected 2 blank lines, found 1

    reviewbot reviewbot

    Col: 80 E501 line too long (82 > 79 characters)

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (3)

    reviewbot reviewbot

    local variable 'e' is assigned to but never used

    reviewbot reviewbot

    Col: 22 E225 missing whitespace around operator

    reviewbot reviewbot

    Col: 80 E501 line too long (100 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (81 > 79 characters)

    reviewbot reviewbot

    Col: 5 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 13 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 9 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 1 E302 expected 2 blank lines, found 1

    reviewbot reviewbot

    Col: 80 E501 line too long (82 > 79 characters)

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (3)

    reviewbot reviewbot

    local variable 'e' is assigned to but never used

    reviewbot reviewbot

    Col: 22 E225 missing whitespace around operator

    reviewbot reviewbot

    Col: 80 E501 line too long (100 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (81 > 79 characters)

    reviewbot reviewbot

    'django_reset' imported but unused

    reviewbot reviewbot

    'from settings_local import *' used; unable to detect undefined names

    reviewbot reviewbot

    'HttpResponseRedirect' imported but unused

    reviewbot reviewbot

    Col: 1 E302 expected 2 blank lines, found 0

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (3)

    reviewbot reviewbot

    Col: 5 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 13 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 9 E265 block comment should start with '# '

    reviewbot reviewbot

    Col: 1 E302 expected 2 blank lines, found 1

    reviewbot reviewbot

    redefinition of unused 'login_required' from line 3

    reviewbot reviewbot

    'render' imported but unused

    reviewbot reviewbot

    Col: 80 E501 line too long (82 > 79 characters)

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (3)

    reviewbot reviewbot

    local variable 'e' is assigned to but never used

    reviewbot reviewbot

    undefined name 'UserSocialAuth'

    reviewbot reviewbot

    Col: 22 E225 missing whitespace around operator

    reviewbot reviewbot

    Col: 80 E501 line too long (100 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (81 > 79 characters)

    reviewbot reviewbot

    'django_reset' imported but unused

    reviewbot reviewbot

    'from settings_local import *' used; unable to detect undefined names

    reviewbot reviewbot

    'HttpResponseRedirect' imported but unused

    reviewbot reviewbot

    redefinition of unused 'login_required' from line 3

    reviewbot reviewbot

    'render' imported but unused

    reviewbot reviewbot

    Col: 48 E225 missing whitespace around operator

    reviewbot reviewbot

    Col: 80 E501 line too long (81 > 79 characters)

    reviewbot reviewbot

    'django_reset' imported but unused

    reviewbot reviewbot

    'from settings_local import *' used; unable to detect undefined names

    reviewbot reviewbot

    Imports should be alphabetized.

    brennie brennie

    redefinition of unused 'login_required' from line 3

    reviewbot reviewbot

    'render' imported but unused

    reviewbot reviewbot

    This would go with the django imports asits a 3rd party import. In general, imports are grouped as such: from …

    brennie brennie

    Undo this.

    brennie brennie

    See other comment about imports.

    brennie brennie

    Only one blank line between class methods.

    brennie brennie

    Col: 80 E501 line too long (80 > 79 characters)

    reviewbot reviewbot

    Col: 80 E501 line too long (81 > 79 characters)

    reviewbot reviewbot

    'django_reset' imported but unused

    reviewbot reviewbot

    These must be added in load_siteconfig in admin/siteconfig.py line 366 with the web api token backend.

    brennie brennie

    These need to be blank and added to the settings_local template.

    brennie brennie

    These need to be blank and added to the settings_local template.

    brennie brennie

    Missing trailing comma.

    brennie brennie

    'from settings_local import *' used; unable to detect undefined names

    reviewbot reviewbot

    Undo this.

    brennie brennie

    We should only render these if a user hasnt already connected with them.

    brennie brennie

    Lets put these under social/ or something. Otherwise they are polluting the global URL namespace.

    brennie brennie

    These would be in settings.py

    brennie brennie

    redefinition of unused 'login_required' from line 3

    reviewbot reviewbot

    'render' imported but unused

    reviewbot reviewbot

    redefinition of unused 'ReviewRequest' from line 51

    reviewbot reviewbot

    redefinition of unused 'User' from line 17

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (2)

    reviewbot reviewbot

    Col: 20 E703 statement ends with a semicolon

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (2)

    reviewbot reviewbot

    Col: 5 E303 too many blank lines (2)

    reviewbot reviewbot

    Col: 27 E225 missing whitespace around operator

    reviewbot reviewbot

    Col: 43 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 45 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 65 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 67 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 80 E501 line too long (132 > 79 characters)

    reviewbot reviewbot

    Col: 81 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 83 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 119 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    Col: 121 E251 unexpected spaces around keyword / parameter equals

    reviewbot reviewbot

    'django_reset' imported but unused

    reviewbot reviewbot

    'from settings_local import *' used; unable to detect undefined names

    reviewbot reviewbot

    Col: 1 E303 too many blank lines (3)

    reviewbot reviewbot

    This file shouldnt be in the diff.

    brennie brennie

    If you insert the other auth backends correctly, this wont be necessary. See my comment from a while back.

    brennie brennie

    Undo.

    brennie brennie

    Blank line between these.

    brennie brennie

    Undo

    brennie brennie

    The password backend kludge stuff needs to be undone.

    brennie brennie

    Undo.

    brennie brennie

    Can you add documentation to explain what this does?

    brennie brennie

    Undo.

    brennie brennie

    Missing a period.

    brennie brennie

    Use a tuple. This list should not be changing at runtime.

    brennie brennie

    Review Board

    brennie brennie

    Single quotes for strings.

    brennie brennie

    Reformat as: raise ValueError( '... %s' % ', '.join(self.PR_STATUSES) )

    brennie brennie

    Reformat as: status_url = self._build_api_url( '%s/statuses/%s % (self._get_repo_api_url(repository), commit) )

    brennie brennie

    Missing trailing comma.

    brennie brennie

    We dont use PEP484. This should be typename, optional, e.g., unicode, optional.

    brennie brennie

    types should use :py:class:`type` You can just say the response status here instead, e.g. "A 200 OK is returned if …

    brennie brennie

    Blank line between these

    brennie brennie

    Blank line between these.

    brennie brennie

    This corresponds to a header, so it should use the header name X-Hub-Signature.

    brennie brennie

    dict. Same elsewhere.

    brennie brennie

    Blank line between these.

    brennie brennie

    This seems like testing code, yes?

    brennie brennie

    Should use an invalid password instead.

    brennie brennie

    You can move that inline.

    brennie brennie

    Undo

    brennie brennie

    Undo.

    brennie brennie

    3rd party stuff should come before reviewboard.

    brennie brennie

    Undo.

    brennie brennie

    Looks like youve had a rebase go bad.

    brennie brennie

    First import should be from __future__ import unicode_literals.

    brennie brennie

    These methods need docstrings.

    brennie brennie

    booleans are singletons in python, you will want to use is not True.

    brennie brennie

    str.format() is quite slow. Use %-interpolation instead.

    brennie brennie

    Reformat as per other comment.

    brennie brennie

    We prefer to format as: return { 'social': social, 'user': social.user, 'new_association': True, } No semicolon necessary.

    brennie brennie

    comments should be complete sentences and end with periods.

    brennie brennie

    'string' imported but unused

    reviewbot reviewbot

    'random.choice' imported but unused

    reviewbot reviewbot

    This shouldn't be in the diff. Whats up with this?

    brennie brennie

    Likewise this also shouldn't be in the diff.

    brennie brennie

    'social.exceptions.AuthCanceled' imported but unused

    reviewbot reviewbot

    undefined name '_'

    reviewbot reviewbot

    undefined name '_'

    reviewbot reviewbot

    undefined name '_'

    reviewbot reviewbot

    'django.http.HttpResponse' imported but unused

    reviewbot reviewbot

    'social.exceptions as social_exceptions' imported but unused

    reviewbot reviewbot

    'djblets.configforms.forms.ConfigPageForm' imported but unused

    reviewbot reviewbot

    'django.contrib.messages' imported but unused

    reviewbot reviewbot

    'django.shortcuts.redirect' imported but unused

    reviewbot reviewbot

    'django.shortcuts.HttpResponseRedirect' imported but unused

    reviewbot reviewbot

    'django.core.urlresolvers.reverse' imported but unused

    reviewbot reviewbot

    'django.conf.urls.include' imported but unused

    reviewbot reviewbot

    'django.http.HttpResponseRedirect' imported but unused

    reviewbot reviewbot

    'django.utils.translation.ugettext_lazy as _' imported but unused

    reviewbot reviewbot

    'django.core.urlresolvers.reverse' imported but unused

    reviewbot reviewbot

    'djblets.util.decorators.augment_method_from' imported but unused

    reviewbot reviewbot

    'djblets.configforms.views.ConfigPagesView' imported but unused

    reviewbot reviewbot

    'reviewboard.admin.decorators.superuser_required' imported but unused

    reviewbot reviewbot
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          docs/manual/fixtures/initial_data.json
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          docs/manual/fixtures/initial_data.json
      
      
    2. reviewboard/accounts/forms/pages.py (Diff revision 1)
       
       
      Show all issues
      Col: 5
       E265 block comment should start with '# '
      
    3. reviewboard/accounts/forms/pages.py (Diff revision 1)
       
       
      Show all issues
      Col: 13
       E265 block comment should start with '# '
      
    4. reviewboard/accounts/forms/pages.py (Diff revision 1)
       
       
      Show all issues
      Col: 9
       E265 block comment should start with '# '
      
    5. reviewboard/accounts/models.py (Diff revision 1)
       
       
      Show all issues
      Col: 1
       E302 expected 2 blank lines, found 1
      
    6. reviewboard/hostingsvcs/github.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (82 > 79 characters)
      
    7. reviewboard/hostingsvcs/github.py (Diff revision 1)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (3)
      
    8. reviewboard/hostingsvcs/github.py (Diff revision 1)
       
       
      Show all issues
       local variable 'e' is assigned to but never used
      
    9. reviewboard/hostingsvcs/github.py (Diff revision 1)
       
       
      Show all issues
      Col: 22
       E225 missing whitespace around operator
      
    10. reviewboard/hostingsvcs/github.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (100 > 79 characters)
      
    11. reviewboard/reviews/builtin_fields.py (Diff revision 1)
       
       
      Show all issues
      Col: 80
       E501 line too long (81 > 79 characters)
      
    12. 
        
    DA
    brennie
    1. 
        
    2. reviewboard/accounts/models.py (Diff revision 1)
       
       
      Show all issues

      We should use social-auth-core and social-auth-app-django for this. See http://python-social-auth.readthedocs.io/en/latest/. We don't want to limit this too just GitHub. Ideally we would able to link to PRs from GitHub, GitLab, Bitbucket, etc. So a generic notion of a service (like social-auth-core provides) is what we're going to want to do.

    3. 
        
    DA
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/settings.py
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          docs/manual/fixtures/initial_data.json
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/settings.py
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          docs/manual/fixtures/initial_data.json
      
      
    2. reviewboard/accounts/forms/pages.py (Diff revision 2)
       
       
      Show all issues
      Col: 5
       E265 block comment should start with '# '
      
    3. reviewboard/accounts/forms/pages.py (Diff revision 2)
       
       
      Show all issues
      Col: 13
       E265 block comment should start with '# '
      
    4. reviewboard/accounts/forms/pages.py (Diff revision 2)
       
       
      Show all issues
      Col: 9
       E265 block comment should start with '# '
      
    5. reviewboard/accounts/models.py (Diff revision 2)
       
       
      Show all issues
      Col: 1
       E302 expected 2 blank lines, found 1
      
    6. reviewboard/hostingsvcs/github.py (Diff revision 2)
       
       
      Show all issues
      Col: 80
       E501 line too long (82 > 79 characters)
      
    7. reviewboard/hostingsvcs/github.py (Diff revision 2)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (3)
      
    8. reviewboard/hostingsvcs/github.py (Diff revision 2)
       
       
      Show all issues
       local variable 'e' is assigned to but never used
      
    9. reviewboard/hostingsvcs/github.py (Diff revision 2)
       
       
      Show all issues
      Col: 22
       E225 missing whitespace around operator
      
    10. reviewboard/hostingsvcs/github.py (Diff revision 2)
       
       
      Show all issues
      Col: 80
       E501 line too long (100 > 79 characters)
      
    11. reviewboard/reviews/builtin_fields.py (Diff revision 2)
       
       
      Show all issues
      Col: 80
       E501 line too long (81 > 79 characters)
      
    12. reviewboard/settings.py (Diff revision 2)
       
       
      Show all issues
       'django_reset' imported but unused
      
    13. reviewboard/settings.py (Diff revision 2)
       
       
      Show all issues
       'from settings_local import *' used; unable to detect undefined names
      
    14. 
        
    DA
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/accounts/urls.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/accounts/urls.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/evolutions/__init__.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/accounts/evolutions/github_username.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
          reviewboard/accounts/admin.py
          reviewboard/accounts/models.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
    2. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
       'HttpResponseRedirect' imported but unused
      
    3. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
      Col: 1
       E302 expected 2 blank lines, found 0
      
    4. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (3)
      
    5. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
      Col: 5
       E265 block comment should start with '# '
      
    6. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
      Col: 13
       E265 block comment should start with '# '
      
    7. reviewboard/accounts/forms/pages.py (Diff revision 3)
       
       
      Show all issues
      Col: 9
       E265 block comment should start with '# '
      
    8. reviewboard/accounts/models.py (Diff revision 3)
       
       
      Show all issues
      Col: 1
       E302 expected 2 blank lines, found 1
      
    9. reviewboard/accounts/views.py (Diff revision 3)
       
       
      Show all issues
       redefinition of unused 'login_required' from line 3
      
    10. reviewboard/accounts/views.py (Diff revision 3)
       
       
      Show all issues
       'render' imported but unused
      
    11. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
      Col: 80
       E501 line too long (82 > 79 characters)
      
    12. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (3)
      
    13. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
       local variable 'e' is assigned to but never used
      
    14. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
       undefined name 'UserSocialAuth'
      
    15. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
      Col: 22
       E225 missing whitespace around operator
      
    16. reviewboard/hostingsvcs/github.py (Diff revision 3)
       
       
      Show all issues
      Col: 80
       E501 line too long (100 > 79 characters)
      
    17. reviewboard/reviews/builtin_fields.py (Diff revision 3)
       
       
      Show all issues
      Col: 80
       E501 line too long (81 > 79 characters)
      
    18. reviewboard/settings.py (Diff revision 3)
       
       
      Show all issues
       'django_reset' imported but unused
      
    19. reviewboard/settings.py (Diff revision 3)
       
       
      Show all issues
       'from settings_local import *' used; unable to detect undefined names
      
    20. 
        
    DA
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
    2. reviewboard/accounts/forms/pages.py (Diff revision 4)
       
       
      Show all issues
       'HttpResponseRedirect' imported but unused
      
    3. reviewboard/accounts/views.py (Diff revision 4)
       
       
      Show all issues
       redefinition of unused 'login_required' from line 3
      
    4. reviewboard/accounts/views.py (Diff revision 4)
       
       
      Show all issues
       'render' imported but unused
      
    5. reviewboard/hostingsvcs/github.py (Diff revision 4)
       
       
      Show all issues
      Col: 48
       E225 missing whitespace around operator
      
    6. reviewboard/reviews/builtin_fields.py (Diff revision 4)
       
       
      Show all issues
      Col: 80
       E501 line too long (81 > 79 characters)
      
    7. reviewboard/settings.py (Diff revision 4)
       
       
      Show all issues
       'django_reset' imported but unused
      
    8. reviewboard/settings.py (Diff revision 4)
       
       
      Show all issues
       'from settings_local import *' used; unable to detect undefined names
      
    9. 
        
    DA
    DA
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/settings.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/accounts/forms/pages.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/github.py
      
      Ignored Files:
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
      
      
    2. reviewboard/accounts/views.py (Diff revision 5)
       
       
      Show all issues
       redefinition of unused 'login_required' from line 3
      
    3. reviewboard/accounts/views.py (Diff revision 5)
       
       
      Show all issues
       'render' imported but unused
      
    4. reviewboard/hostingsvcs/github.py (Diff revision 5)
       
       
      Show all issues
      Col: 80
       E501 line too long (80 > 79 characters)
      
    5. reviewboard/reviews/builtin_fields.py (Diff revision 5)
       
       
      Show all issues
      Col: 80
       E501 line too long (81 > 79 characters)
      
    6. reviewboard/settings.py (Diff revision 5)
       
       
      Show all issues
       'django_reset' imported but unused
      
    7. reviewboard/settings.py (Diff revision 5)
       
       
      Show all issues
       'from settings_local import *' used; unable to detect undefined names
      
    8. 
        
    brennie
    1. 
        
    2. reviewboard/accounts/forms/pages.py (Diff revision 5)
       
       
      Show all issues

      Imports should be alphabetized.

    3. reviewboard/hostingsvcs/github.py (Diff revision 5)
       
       
      Show all issues

      This would go with the django imports asits a 3rd party import.

      In general, imports are grouped as such:

      from __future__ import unicode_literals
      
      import os  # Standard library
      
      import django  # 3rd Party
      
      import reviewboard  # The current repo.
      
    4. reviewboard/hostingsvcs/github.py (Diff revision 5)
       
       
      Show all issues

      Undo this.

    5. reviewboard/hostingsvcs/github.py (Diff revision 5)
       
       
      Show all issues

      See other comment about imports.

    6. reviewboard/hostingsvcs/github.py (Diff revision 5)
       
       
      Show all issues

      Only one blank line between class methods.

    7. reviewboard/settings.py (Diff revision 5)
       
       
       
      Show all issues

      These must be added in load_siteconfig in admin/siteconfig.py line 366 with the web api token backend.

    8. reviewboard/settings.py (Diff revision 5)
       
       
       
      Show all issues

      These need to be blank and added to the settings_local template.

    9. reviewboard/settings.py (Diff revision 5)
       
       
       
      Show all issues

      These need to be blank and added to the settings_local template.

    10. reviewboard/settings.py (Diff revision 5)
       
       
      Show all issues

      Missing trailing comma.

    11. reviewboard/settings.py (Diff revision 5)
       
       
      Show all issues

      Undo this.

    12. Show all issues

      We should only render these if a user hasnt already connected with them.

    13. reviewboard/urls.py (Diff revision 5)
       
       
      Show all issues

      Lets put these under social/ or something. Otherwise they are polluting the global URL namespace.

    14. reviewboard/urls.py (Diff revision 5)
       
       
       
       
       
       
      Show all issues

      These would be in settings.py

    15. 
        
    chipx86
    1. Can you attach a screenshot of the configuration forms?

    2. 
        
    DA
    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/reviews/models/base_comment.py
          reviewboard/reviews/evolutions/__init__.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/reviews/evolutions/pull_request.py
          reviewboard/accounts/forms/pages.py
          reviewboard/diffviewer/diffutils.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/tests/test_github.py
          reviewboard/hostingsvcs/service.py
          reviewboard/settings.py
          reviewboard/hostingsvcs/github.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/models/review.py
      
      Ignored Files:
          reviewboard/templates/base/_nav_support_menu.html
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
          reviewboard/templates/accounts/claim_github.html
      
      
    2. reviewboard/accounts/views.py (Diff revision 6)
       
       
      Show all issues
       redefinition of unused 'login_required' from line 3
      
    3. reviewboard/accounts/views.py (Diff revision 6)
       
       
      Show all issues
       'render' imported but unused
      
    4. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
       redefinition of unused 'ReviewRequest' from line 51
      
    5. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
       redefinition of unused 'User' from line 17
      
    6. reviewboard/settings.py (Diff revision 6)
       
       
      Show all issues
       'django_reset' imported but unused
      
    7. reviewboard/settings.py (Diff revision 6)
       
       
      Show all issues
       'from settings_local import *' used; unable to detect undefined names
      
    8. 
        
    reviewbot
    1. Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/hostingsvcs/gitlab.py
          reviewboard/reviews/models/base_comment.py
          reviewboard/reviews/evolutions/__init__.py
          reviewboard/accounts/views.py
          reviewboard/accounts/urls.py
          reviewboard/accounts/pages.py
          reviewboard/reviews/evolutions/pull_request.py
          reviewboard/accounts/forms/pages.py
          reviewboard/diffviewer/diffutils.py
          reviewboard/reviews/builtin_fields.py
          reviewboard/urls.py
          reviewboard/hostingsvcs/tests/test_github.py
          reviewboard/hostingsvcs/service.py
          reviewboard/settings.py
          reviewboard/hostingsvcs/github.py
          reviewboard/reviews/models/review_request.py
          reviewboard/reviews/models/review.py
      
      Ignored Files:
          reviewboard/templates/base/_nav_support_menu.html
          reviewboard/templates/accounts/external_service_auth.html
          docs/manual/fixtures/initial_data.json
          reviewboard/templates/accounts/claim_github.html
      
      
    2. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (2)
      
    3. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 20
       E703 statement ends with a semicolon
      
    4. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (2)
      
    5. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 5
       E303 too many blank lines (2)
      
    6. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 27
       E225 missing whitespace around operator
      
    7. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 43
       E251 unexpected spaces around keyword / parameter equals
      
    8. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 45
       E251 unexpected spaces around keyword / parameter equals
      
    9. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 65
       E251 unexpected spaces around keyword / parameter equals
      
    10. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 67
       E251 unexpected spaces around keyword / parameter equals
      
    11. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 80
       E501 line too long (132 > 79 characters)
      
    12. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 81
       E251 unexpected spaces around keyword / parameter equals
      
    13. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 83
       E251 unexpected spaces around keyword / parameter equals
      
    14. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 119
       E251 unexpected spaces around keyword / parameter equals
      
    15. reviewboard/hostingsvcs/github.py (Diff revision 6)
       
       
      Show all issues
      Col: 121
       E251 unexpected spaces around keyword / parameter equals
      
    16. reviewboard/settings.py (Diff revision 6)
       
       
      Show all issues
      Col: 1
       E303 too many blank lines (3)
      
    17. 
        
    DA
    brennie
    1. 
        
    2. docs/manual/fixtures/initial_data.json (Diff revision 7)
       
       
      Show all issues

      This file shouldnt be in the diff.

    3. reviewboard/accounts/backends.py (Diff revision 7)
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      If you insert the other auth backends correctly, this wont be necessary. See my comment from a while back.

    4. reviewboard/accounts/forms/pages.py (Diff revision 7)
       
       
      Show all issues

      Undo.

    5. reviewboard/accounts/forms/pages.py (Diff revision 7)
       
       
       
      Show all issues

      Blank line between these.

    6. reviewboard/accounts/forms/pages.py (Diff revision 7)
       
       
       
      Show all issues

      Undo

    7. reviewboard/accounts/forms/pages.py (Diff revision 7)
       
       
      Show all issues

      The password backend kludge stuff needs to be undone.

    8. reviewboard/accounts/forms/pages.py (Diff revision 7)
       
       
      Show all issues

      Undo.

    9. reviewboard/accounts/middleware.py (Diff revision 7)
       
       
       
       
       
       
       
       
      Show all issues

      Can you add documentation to explain what this does?

    10. reviewboard/diffviewer/diffutils.py (Diff revision 7)
       
       
      Show all issues

      Undo.

    11. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      Missing a period.

    12. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      Use a tuple. This list should not be changing at runtime.

    13. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      Review Board

    14. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      Single quotes for strings.

    15. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      Reformat as:

      raise ValueError(
          '... %s'
          % ', '.join(self.PR_STATUSES)
      )
      
    16. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
       
       
      Show all issues

      Reformat as:

      status_url = self._build_api_url(
          '%s/statuses/%s
          % (self._get_repo_api_url(repository), commit)
      )
      
    17. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      Missing trailing comma.

    18. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
       
       
       
       
      Show all issues

      We dont use PEP484. This should be typename, optional, e.g., unicode, optional.

    19. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      types should use :py:class:`type`

      You can just say the response status here instead, e.g.

      "A 200 OK is returned if the review request was updated successfully. Otherwise a 400 Bad Request is returned."

    20. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      Blank line between these

    21. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      Blank line between these.

    22. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      This corresponds to a header, so it should use the header name X-Hub-Signature.

    23. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      dict. Same elsewhere.

    24. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
      Show all issues

      Blank line between these.

    25. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
       
      Show all issues

      This seems like testing code, yes?

    26. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
      Show all issues

      Should use an invalid password instead.

    27. reviewboard/hostingsvcs/github.py (Diff revision 7)
       
       
       
       
      Show all issues

      You can move that inline.

    28. reviewboard/hostingsvcs/gitlab.py (Diff revision 7)
       
       
      Show all issues

      Undo

    29. reviewboard/settings.py (Diff revision 7)
       
       
      Show all issues

      Undo.

    30. reviewboard/settings.py (Diff revision 7)
       
       
      Show all issues

      3rd party stuff should come before reviewboard.

    31. reviewboard/settings.py (Diff revision 7)
       
       
      Show all issues

      Undo.

    32. reviewboard/settings.py (Diff revision 7)
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Looks like youve had a rebase go bad.

      1. Nevermind I had less than enough coffee when I opened this issue.

    33. reviewboard/social/pipeline.py (Diff revision 7)
       
       
      Show all issues

      First import should be from __future__ import unicode_literals.

    34. reviewboard/social/pipeline.py (Diff revision 7)
       
       
      Show all issues

      These methods need docstrings.

    35. reviewboard/social/pipeline.py (Diff revision 7)
       
       
      Show all issues

      booleans are singletons in python, you will want to use is not True.

    36. reviewboard/social/pipeline.py (Diff revision 7)
       
       
      Show all issues

      str.format() is quite slow. Use %-interpolation instead.

    37. reviewboard/social/pipeline.py (Diff revision 7)
       
       
       
       
       
       
      Show all issues

      Reformat as per other comment.

    38. reviewboard/social/pipeline.py (Diff revision 7)
       
       
       
       
      Show all issues

      We prefer to format as:

      return {
          'social': social,
          'user': social.user,
          'new_association': True,
      }
      

      No semicolon necessary.

    39. reviewboard/social/pipeline.py (Diff revision 7)
       
       
       
      Show all issues

      comments should be complete sentences and end with periods.

    40. 
        
    DA
    Review request changed
    Summary:
    Create a new Review Request after creating a PR on Github - duplicate [WIP]
    Create a new Review Request after creating a PR on Github
    Commit:
    76e0a1a01a0f3dd0e377df7941354b94a01db9dc
    2288ef1ac25f2cfbcd5b0194e957ace6dc86b97e

    Checks run (1 failed, 1 succeeded, 1 failed with error)

    JSHint passed.
    PEP8 Style Checker internal error.
    Pyflakes failed.

    Pyflakes

    brennie
    1. 
        
    2. Show all issues

      There's some weird stuff going on in settings.py. It looks like you may have squashed some changes into this one.

    3. reviewboard/settings.py (Diff revision 8)
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      This shouldn't be in the diff. Whats up with this?

    4. reviewboard/settings.py (Diff revision 8)
       
       
       
       
       
       
       
       
       
       
       
       
       
      Show all issues

      Likewise this also shouldn't be in the diff.

    5. 
        
    DA
    DA
    Review request changed
    Commit:
    2214898a7f6d2eba433d0a36b0a56a8f40917c5c
    54f825b56adeb7a907409198edfd78a59fadb1d5

    Checks run (1 failed, 1 succeeded, 1 failed with error)

    JSHint passed.
    PEP8 Style Checker internal error.
    Pyflakes failed.

    Pyflakes

    DA
    Review request changed
    Commit:
    54f825b56adeb7a907409198edfd78a59fadb1d5
    e0cfbdab2219847f8240813fb3876c4c2dd9d02f

    Checks run (1 failed, 1 succeeded, 1 failed with error)

    JSHint passed.
    PEP8 Style Checker internal error.
    Pyflakes failed.

    Pyflakes

    david
    Review request changed
    Status:
    Discarded