Create a new Review Request after creating a PR on Github

Review Request #8741 - Created Feb. 12, 2017 and updated

Daniel Bak
Review Board
dvcs
e0cfbda...
reviewboard, students

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.

  • 21
  • 0
  • 111
  • 17
  • 149
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. Barret Rennie Barret Rennie
Can you add documentation to explain what this does? Barret Rennie Barret Rennie
This shouldn't be in the diff. Whats up with this? Barret Rennie Barret Rennie
Likewise this also shouldn't be in the diff. Barret Rennie Barret Rennie
undefined name '_' Review Bot Review Bot
undefined name '_' Review Bot Review Bot
undefined name '_' Review Bot Review Bot
'django.http.HttpResponse' imported but unused Review Bot Review Bot
'social.exceptions as social_exceptions' imported but unused Review Bot Review Bot
'djblets.configforms.forms.ConfigPageForm' imported but unused Review Bot Review Bot
'django.contrib.messages' imported but unused Review Bot Review Bot
'django.shortcuts.redirect' imported but unused Review Bot Review Bot
'django.shortcuts.HttpResponseRedirect' imported but unused Review Bot Review Bot
'django.core.urlresolvers.reverse' imported but unused Review Bot Review Bot
'django.conf.urls.include' imported but unused Review Bot Review Bot
'django.http.HttpResponseRedirect' imported but unused Review Bot Review Bot
'django.utils.translation.ugettext_lazy as _' imported but unused Review Bot Review Bot
'django.core.urlresolvers.reverse' imported but unused Review Bot Review Bot
'djblets.util.decorators.augment_method_from' imported but unused Review Bot Review Bot
'djblets.configforms.views.ConfigPagesView' imported but unused Review Bot Review Bot
'reviewboard.admin.decorators.superuser_required' imported but unused Review Bot Review Bot
Review Bot
  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)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  3. reviewboard/accounts/forms/pages.py (Diff revision 1)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  4. reviewboard/accounts/forms/pages.py (Diff revision 1)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  5. reviewboard/accounts/models.py (Diff revision 1)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  6. reviewboard/hostingsvcs/github.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. reviewboard/hostingsvcs/github.py (Diff revision 1)
     
     
    Col: 5
     E303 too many blank lines (3)
    
  8. reviewboard/hostingsvcs/github.py (Diff revision 1)
     
     
     local variable 'e' is assigned to but never used
    
  9. reviewboard/hostingsvcs/github.py (Diff revision 1)
     
     
    Col: 22
     E225 missing whitespace around operator
    
  10. reviewboard/hostingsvcs/github.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (100 > 79 characters)
    
  11. reviewboard/reviews/builtin_fields.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  12. 
      
Daniel Bak
Barret Rennie
  1. 
      
  2. reviewboard/accounts/models.py (Diff revision 1)
     
     

    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. 
      
Daniel Bak
Review Bot
  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)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  3. reviewboard/accounts/forms/pages.py (Diff revision 2)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  4. reviewboard/accounts/forms/pages.py (Diff revision 2)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  5. reviewboard/accounts/models.py (Diff revision 2)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  6. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  7. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
    Col: 5
     E303 too many blank lines (3)
    
  8. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
     local variable 'e' is assigned to but never used
    
  9. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
    Col: 22
     E225 missing whitespace around operator
    
  10. reviewboard/hostingsvcs/github.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (100 > 79 characters)
    
  11. reviewboard/reviews/builtin_fields.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  12. reviewboard/settings.py (Diff revision 2)
     
     
     'django_reset' imported but unused
    
  13. reviewboard/settings.py (Diff revision 2)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  14. 
      
Daniel Bak
Review Bot
  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)
     
     
     'HttpResponseRedirect' imported but unused
    
  3. reviewboard/accounts/forms/pages.py (Diff revision 3)
     
     
    Col: 1
     E302 expected 2 blank lines, found 0
    
  4. reviewboard/accounts/forms/pages.py (Diff revision 3)
     
     
    Col: 5
     E303 too many blank lines (3)
    
  5. reviewboard/accounts/forms/pages.py (Diff revision 3)
     
     
    Col: 5
     E265 block comment should start with '# '
    
  6. reviewboard/accounts/forms/pages.py (Diff revision 3)
     
     
    Col: 13
     E265 block comment should start with '# '
    
  7. reviewboard/accounts/forms/pages.py (Diff revision 3)
     
     
    Col: 9
     E265 block comment should start with '# '
    
  8. reviewboard/accounts/models.py (Diff revision 3)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  9. reviewboard/accounts/views.py (Diff revision 3)
     
     
     redefinition of unused 'login_required' from line 3
    
  10. reviewboard/accounts/views.py (Diff revision 3)
     
     
     'render' imported but unused
    
  11. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (82 > 79 characters)
    
  12. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
    Col: 5
     E303 too many blank lines (3)
    
  13. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
     local variable 'e' is assigned to but never used
    
  14. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
     undefined name 'UserSocialAuth'
    
  15. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
    Col: 22
     E225 missing whitespace around operator
    
  16. reviewboard/hostingsvcs/github.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (100 > 79 characters)
    
  17. reviewboard/reviews/builtin_fields.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  18. reviewboard/settings.py (Diff revision 3)
     
     
     'django_reset' imported but unused
    
  19. reviewboard/settings.py (Diff revision 3)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  20. 
      
Daniel Bak
Review Bot
  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)
     
     
     'HttpResponseRedirect' imported but unused
    
  3. reviewboard/accounts/views.py (Diff revision 4)
     
     
     redefinition of unused 'login_required' from line 3
    
  4. reviewboard/accounts/views.py (Diff revision 4)
     
     
     'render' imported but unused
    
  5. reviewboard/hostingsvcs/github.py (Diff revision 4)
     
     
    Col: 48
     E225 missing whitespace around operator
    
  6. reviewboard/reviews/builtin_fields.py (Diff revision 4)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  7. reviewboard/settings.py (Diff revision 4)
     
     
     'django_reset' imported but unused
    
  8. reviewboard/settings.py (Diff revision 4)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  9. 
      
Daniel Bak
Daniel Bak
Review Bot
  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)
     
     
     redefinition of unused 'login_required' from line 3
    
  3. reviewboard/accounts/views.py (Diff revision 5)
     
     
     'render' imported but unused
    
  4. reviewboard/hostingsvcs/github.py (Diff revision 5)
     
     
    Col: 80
     E501 line too long (80 > 79 characters)
    
  5. reviewboard/reviews/builtin_fields.py (Diff revision 5)
     
     
    Col: 80
     E501 line too long (81 > 79 characters)
    
  6. reviewboard/settings.py (Diff revision 5)
     
     
     'django_reset' imported but unused
    
  7. reviewboard/settings.py (Diff revision 5)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  8. 
      
Barret Rennie
  1. 
      
  2. reviewboard/accounts/forms/pages.py (Diff revision 5)
     
     

    Imports should be alphabetized.

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

    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)
     
     

    Undo this.

  5. reviewboard/hostingsvcs/github.py (Diff revision 5)
     
     

    See other comment about imports.

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

    Only one blank line between class methods.

  7. reviewboard/settings.py (Diff revision 5)
     
     
     

    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)
     
     
     

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

  9. reviewboard/settings.py (Diff revision 5)
     
     
     

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

  10. reviewboard/settings.py (Diff revision 5)
     
     

    Missing trailing comma.

  11. reviewboard/settings.py (Diff revision 5)
     
     

    Undo this.

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

  13. reviewboard/urls.py (Diff revision 5)
     
     

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

  14. reviewboard/urls.py (Diff revision 5)
     
     
     
     
     
     

    These would be in settings.py

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

  2. 
      
Daniel Bak
Review Bot
  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)
     
     
     redefinition of unused 'login_required' from line 3
    
  3. reviewboard/accounts/views.py (Diff revision 6)
     
     
     'render' imported but unused
    
  4. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
     redefinition of unused 'ReviewRequest' from line 51
    
  5. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
     redefinition of unused 'User' from line 17
    
  6. reviewboard/settings.py (Diff revision 6)
     
     
     'django_reset' imported but unused
    
  7. reviewboard/settings.py (Diff revision 6)
     
     
     'from settings_local import *' used; unable to detect undefined names
    
  8. 
      
Review Bot
  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)
     
     
    Col: 5
     E303 too many blank lines (2)
    
  3. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 20
     E703 statement ends with a semicolon
    
  4. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 5
     E303 too many blank lines (2)
    
  5. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 5
     E303 too many blank lines (2)
    
  6. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 27
     E225 missing whitespace around operator
    
  7. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 43
     E251 unexpected spaces around keyword / parameter equals
    
  8. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 45
     E251 unexpected spaces around keyword / parameter equals
    
  9. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 65
     E251 unexpected spaces around keyword / parameter equals
    
  10. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 67
     E251 unexpected spaces around keyword / parameter equals
    
  11. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 80
     E501 line too long (132 > 79 characters)
    
  12. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 81
     E251 unexpected spaces around keyword / parameter equals
    
  13. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 83
     E251 unexpected spaces around keyword / parameter equals
    
  14. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 119
     E251 unexpected spaces around keyword / parameter equals
    
  15. reviewboard/hostingsvcs/github.py (Diff revision 6)
     
     
    Col: 121
     E251 unexpected spaces around keyword / parameter equals
    
  16. reviewboard/settings.py (Diff revision 6)
     
     
    Col: 1
     E303 too many blank lines (3)
    
  17. 
      
Daniel Bak
Barret Rennie
  1. 
      
  2. docs/manual/fixtures/initial_data.json (Diff revision 7)
     
     

    This file shouldnt be in the diff.

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

    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)
     
     
     

    Blank line between these.

  5. reviewboard/accounts/forms/pages.py (Diff revision 7)
     
     
     
  6. reviewboard/accounts/forms/pages.py (Diff revision 7)
     
     

    The password backend kludge stuff needs to be undone.

  7. reviewboard/accounts/middleware.py (Diff revision 7)
     
     
     
     
     
     
     
     

    Can you add documentation to explain what this does?

  8. reviewboard/hostingsvcs/github.py (Diff revision 7)
     
     

    Missing a period.

  9. reviewboard/hostingsvcs/github.py (Diff revision 7)
     
     
     

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

  10. reviewboard/hostingsvcs/github.py (Diff revision 7)
     
     

    Review Board

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

    Single quotes for strings.

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

    Reformat as:

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

    Reformat as:

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

    Missing trailing comma.

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

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

  16. reviewboard/hostingsvcs/github.py (Diff revision 7)
     
     
     

    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."

  17. reviewboard/hostingsvcs/github.py (Diff revision 7)
     
     
     

    Blank line between these

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

    Blank line between these.

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

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

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

    dict. Same elsewhere.

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

    Blank line between these.

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

    This seems like testing code, yes?

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

    Should use an invalid password instead.

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

    You can move that inline.

  25. reviewboard/hostingsvcs/gitlab.py (Diff revision 7)
     
     
  26. reviewboard/settings.py (Diff revision 7)
     
     
  27. reviewboard/settings.py (Diff revision 7)
     
     

    3rd party stuff should come before reviewboard.

  28. reviewboard/settings.py (Diff revision 7)
     
     
  29. reviewboard/settings.py (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Looks like youve had a rebase go bad.

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

  30. reviewboard/social/pipeline.py (Diff revision 7)
     
     

    First import should be from __future__ import unicode_literals.

  31. reviewboard/social/pipeline.py (Diff revision 7)
     
     

    These methods need docstrings.

  32. reviewboard/social/pipeline.py (Diff revision 7)
     
     

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

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

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

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

    Reformat as per other comment.

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

    We prefer to format as:

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

    No semicolon necessary.

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

    comments should be complete sentences and end with periods.

  37. 
      
Daniel Bak
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

Diff:

Revision 8 (+1002 -3933)

Show changes

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

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

Pyflakes

Barret Rennie
  1. 
      
  2. 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)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

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

  4. reviewboard/settings.py (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     
     
     

    Likewise this also shouldn't be in the diff.

  5. 
      
Daniel Bak
Daniel Bak
Review request changed

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

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

Pyflakes

Daniel Bak
Review request changed

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

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

Pyflakes

Loading...