Create a new Review Request after creating a PR on Github

Review Request #8741 — Created Feb. 13, 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.

brenniebrennie

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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 …

brenniebrennie

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

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

reviewbotreviewbot

Col: 5 E303 too many blank lines (3)

reviewbotreviewbot

local variable 'e' is assigned to but never used

reviewbotreviewbot

Col: 22 E225 missing whitespace around operator

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

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

reviewbotreviewbot

Col: 5 E303 too many blank lines (3)

reviewbotreviewbot

local variable 'e' is assigned to but never used

reviewbotreviewbot

Col: 22 E225 missing whitespace around operator

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'HttpResponseRedirect' imported but unused

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 0

reviewbotreviewbot

Col: 5 E303 too many blank lines (3)

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

redefinition of unused 'login_required' from line 3

reviewbotreviewbot

'render' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

Col: 5 E303 too many blank lines (3)

reviewbotreviewbot

local variable 'e' is assigned to but never used

reviewbotreviewbot

undefined name 'UserSocialAuth'

reviewbotreviewbot

Col: 22 E225 missing whitespace around operator

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'HttpResponseRedirect' imported but unused

reviewbotreviewbot

redefinition of unused 'login_required' from line 3

reviewbotreviewbot

'render' imported but unused

reviewbotreviewbot

Col: 48 E225 missing whitespace around operator

reviewbotreviewbot

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

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

Imports should be alphabetized.

brenniebrennie

redefinition of unused 'login_required' from line 3

reviewbotreviewbot

'render' imported but unused

reviewbotreviewbot

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

brenniebrennie

Undo this.

brenniebrennie

See other comment about imports.

brenniebrennie

Only one blank line between class methods.

brenniebrennie

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

reviewbotreviewbot

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

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

brenniebrennie

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

brenniebrennie

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

brenniebrennie

Missing trailing comma.

brenniebrennie

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

reviewbotreviewbot

Undo this.

brenniebrennie

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

brenniebrennie

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

brenniebrennie

These would be in settings.py

brenniebrennie

redefinition of unused 'login_required' from line 3

reviewbotreviewbot

'render' imported but unused

reviewbotreviewbot

redefinition of unused 'ReviewRequest' from line 51

reviewbotreviewbot

redefinition of unused 'User' from line 17

reviewbotreviewbot

Col: 5 E303 too many blank lines (2)

reviewbotreviewbot

Col: 20 E703 statement ends with a semicolon

reviewbotreviewbot

Col: 5 E303 too many blank lines (2)

reviewbotreviewbot

Col: 5 E303 too many blank lines (2)

reviewbotreviewbot

Col: 27 E225 missing whitespace around operator

reviewbotreviewbot

Col: 43 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 45 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 65 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 67 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

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

reviewbotreviewbot

Col: 81 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 83 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 119 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

Col: 121 E251 unexpected spaces around keyword / parameter equals

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

Col: 1 E303 too many blank lines (3)

reviewbotreviewbot

This file shouldnt be in the diff.

brenniebrennie

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

brenniebrennie

Undo.

brenniebrennie

Blank line between these.

brenniebrennie

Undo

brenniebrennie

The password backend kludge stuff needs to be undone.

brenniebrennie

Undo.

brenniebrennie

Can you add documentation to explain what this does?

brenniebrennie

Undo.

brenniebrennie

Missing a period.

brenniebrennie

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

brenniebrennie

Review Board

brenniebrennie

Single quotes for strings.

brenniebrennie

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

brenniebrennie

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

brenniebrennie

Missing trailing comma.

brenniebrennie

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

brenniebrennie

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

brenniebrennie

Blank line between these

brenniebrennie

Blank line between these.

brenniebrennie

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

brenniebrennie

dict. Same elsewhere.

brenniebrennie

Blank line between these.

brenniebrennie

This seems like testing code, yes?

brenniebrennie

Should use an invalid password instead.

brenniebrennie

You can move that inline.

brenniebrennie

Undo

brenniebrennie

Undo.

brenniebrennie

3rd party stuff should come before reviewboard.

brenniebrennie

Undo.

brenniebrennie

Looks like youve had a rebase go bad.

brenniebrennie

First import should be from __future__ import unicode_literals.

brenniebrennie

These methods need docstrings.

brenniebrennie

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

brenniebrennie

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

brenniebrennie

Reformat as per other comment.

brenniebrennie

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

brenniebrennie

comments should be complete sentences and end with periods.

brenniebrennie

'string' imported but unused

reviewbotreviewbot

'random.choice' imported but unused

reviewbotreviewbot

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

brenniebrennie

Likewise this also shouldn't be in the diff.

brenniebrennie

'social.exceptions.AuthCanceled' imported but unused

reviewbotreviewbot

undefined name '_'

reviewbotreviewbot

undefined name '_'

reviewbotreviewbot

undefined name '_'

reviewbotreviewbot

'django.http.HttpResponse' imported but unused

reviewbotreviewbot

'social.exceptions as social_exceptions' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'django.contrib.messages' imported but unused

reviewbotreviewbot

'django.shortcuts.redirect' imported but unused

reviewbotreviewbot

'django.shortcuts.HttpResponseRedirect' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

'django.http.HttpResponseRedirect' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot

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

reviewbotreviewbot
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

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

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

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

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

Pyflakes

DA
Review request changed

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

Loading...