Add the avatar services registry

Review Request #7809 — Created Dec. 15, 2015 and submitted

Information

Review Board
release-2.6.x

Reviewers

The avatar services registry is a new object for managing avatar
services, which are defined to provide Review Board multiple ways of
displaying user avatars. Currently, the only supported avatar service
is the Gravatar (http://gravatar.com) service, which is what Review
Board supported previously.

This patch, however, does not migrate to use the new avatar services.

Generated serach indexes have also been added to .gitignore.

Ran unit tests.
Used this in an upcoming patch.

Description From Last Updated

Should use ugettext_lazy

daviddavid

Docstring?

daviddavid

Since we render with a size specified, we should probably include width and height parameters in this tag. Also, can …

daviddavid

We should mark_safe this.

daviddavid

Col: 25 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 29 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 19 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 17 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 32 E241 multiple spaces after ':'

reviewbotreviewbot

I know it wasn't already, but can we alphabetize this list?

daviddavid

Perhaps we should rename these so they start with avatar_service so they all bunch together?

daviddavid

Let's go into a little more detail on this, at the very least saying "avatar services," but ideally clarifying what …

chipx86chipx86

Swap these.

chipx86chipx86

Ordering is backwards.

chipx86chipx86

Similar comment to my other review. We should have a capital letter after the ":", since that's typically how errors …

chipx86chipx86

Blank line between these.

chipx86chipx86

A property should generally have the same type as both the getter and setter. In this case, we're using a …

chipx86chipx86

~ before the class path?

chipx86chipx86

Localized?

chipx86chipx86

Seeing a lot of inconsistency with the class paths in the docs. Some say reviewboard.accounts.avatars.services, some reviewboard.avatars.service, and some reviewboard.avatars.services.

chipx86chipx86

Might wrap better as: raise ItemLookupError( '...' % service) Same below. Also, these should be localized.

chipx86chipx86

I'm not sure it's clear what this really means, as written. Same below. Can we say what saving means, or …

chipx86chipx86

Sort of hitting a parse error with this sentence.

chipx86chipx86

Not necessarily true. Maybe: Whether the registry of avatars was newly populated by this call. If ``False``, the registry was …

chipx86chipx86

We know we're saving below, so probably no need to do that here.

chipx86chipx86

Should pass the service ID as a parameter, rather than a format string.

chipx86chipx86

"set"/"unsetting" doesn't mean a lot outside of the internals of the registries, I don't think. Maybe we can reword this?

chipx86chipx86

This is an internal detail I'd really rather not expose. It's cheap to fetch the SiteConfiguration, since we cache it, …

chipx86chipx86

Since this is something that developers will be interacting with, this should go into a lot more detail on what …

chipx86chipx86

This should go into more detail on what's expected and what's provided to the template.

chipx86chipx86

How about "The user whose avatar URL will be retrieved."

chipx86chipx86

Let's add "in pixels," and that this will be applied to both the width and the height.

chipx86chipx86

"... to HTML." This should go into more detail about how it's rendered (using template), and that it can be …

chipx86chipx86

Let's cache these on the request, taking into account the user ID and size. This will give us a nice …

chipx86chipx86

Let's have this in its own file. Refer to my thoughts at the top about directory structure.

chipx86chipx86

Two blank lines.

chipx86chipx86

This is kind of oddly written, and doesn't make it clear that service_id can impact this. This should be more …

chipx86chipx86

Let's pass the values as keyword arguments, in case we extend down the road.

chipx86chipx86

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 25 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 19 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 29 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 32 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 17 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 39 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 32 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 25 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 19 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 29 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 17 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Two blank lines.

chipx86chipx86

This needs more detail on what this does and how it works, for people not familiar with what we're doing.

chipx86chipx86

This shouldn't have to be called here. It's already managed by the middleware and app initialize stuff, and when setting …

chipx86chipx86

This makes me nervous. Can we just set the things we need?

chipx86chipx86

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'PIPELINE_CSS' imported but unused

reviewbotreviewbot

'PIPELINE_JS' imported but unused

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Things are misaligned here. Can we just shift this to be pep-8 style (even though it's not quite as pretty …

daviddavid

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 24 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 25 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 23 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 19 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 29 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 30 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 31 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 26 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 28 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 17 E241 multiple spaces after ':'

reviewbotreviewbot

Col: 22 E241 multiple spaces after ':'

reviewbotreviewbot

This should all fit on one line.

daviddavid

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'PIPELINE_CSS' imported but unused

reviewbotreviewbot

'PIPELINE_JS' imported but unused

reviewbotreviewbot

'django_reset' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

'PIPELINE_CSS' imported but unused

reviewbotreviewbot

'PIPELINE_JS' imported but unused

reviewbotreviewbot
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/avatars/tests.py
        reviewboard/accounts/avatars/registry.py
        reviewboard/admin/siteconfig.py
        reviewboard/accounts/avatars/services.py
        reviewboard/accounts/avatars/errors.py
        reviewboard/accounts/templatetags/avatars.py
    
    Ignored Files:
        reviewboard/accounts/avatars/__init__.py
        reviewboard/accounts/templatetags/__init__.py
        .gitignore
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/avatars/tests.py
        reviewboard/accounts/avatars/registry.py
        reviewboard/admin/siteconfig.py
        reviewboard/accounts/avatars/services.py
        reviewboard/accounts/avatars/errors.py
        reviewboard/accounts/templatetags/avatars.py
    
    Ignored Files:
        reviewboard/accounts/avatars/__init__.py
        reviewboard/accounts/templatetags/__init__.py
        .gitignore
    
    
  2. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 25
     E241 multiple spaces after ':'
    
  3. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 29
     E241 multiple spaces after ':'
    
  4. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 19
     E241 multiple spaces after ':'
    
  5. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  6. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  7. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  8. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  9. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  10. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  11. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  12. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  13. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  14. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 17
     E241 multiple spaces after ':'
    
  15. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  16. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  17. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  18. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  19. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  20. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  21. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  22. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues
    Col: 32
     E241 multiple spaces after ':'
    
  23. 
      
david
  1. 
      
  2. Show all issues

    Should use ugettext_lazy

  3. Show all issues

    Docstring?

  4. Show all issues

    Since we render with a size specified, we should probably include width and height parameters in this tag.

    Also, can you double check that things are properly escaped when rendering this?

  5. Show all issues

    We should mark_safe this.

    1. Template.render returns a SafeText.

  6. reviewboard/admin/siteconfig.py (Diff revision 1)
     
     
    Show all issues

    I know it wasn't already, but can we alphabetize this list?

  7. 
      
brennie
david
  1. 
      
  2. reviewboard/admin/siteconfig.py (Diff revisions 1 - 2)
     
     
     
     
    Show all issues

    Perhaps we should rename these so they start with avatar_service so they all bunch together?

  3. 
      
chipx86
  1. The directory structure's feeling a bit deep. I get that avatars relate to accounts, but they're not necessarily a part of an account. I can see, for instance, us enhancing this later to let us get an avatar for a remote service, like a GitHub account, without having a local account. Maybe.

    Anyway, room for debate here, but I feel like reviewboard.avatars is a fine namespace for this. Allows this to really be solely about avatars, without needing to be, like, a utility service of accounts (which the current directory structure implies).

    It also allows us to more easily break services.py up into services/base.py and services/gravatar.py. That'll be good down the road, in case we want to add some other built-in services (github.py, custom_url.py, or whatever).

  2. reviewboard/accounts/avatars/errors.py (Diff revision 2)
     
     
    Show all issues

    Let's go into a little more detail on this, at the very least saying "avatar services," but ideally clarifying what this means more.

  3. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    Swap these.

  4. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
     
    Show all issues

    Ordering is backwards.

  5. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    Similar comment to my other review. We should have a capital letter after the ":", since that's typically how errors will be represented (as another exception's message is often used as the "reason" for an error here).

    We also usually have a form more like:

    Could not register the avatar service %(item)s: This service is already registered.

    Also, this one is missing a trailing period.

  6. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    Blank line between these.

    1. Why?

    2. Most of our code follows that. Not all, but most. So primarily, consistency.

  7. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    A property should generally have the same type as both the getter and setter. In this case, we're using a set value of a list, and a get of a generator. I feel consistency is preferable here, particularly since we're never going to really have more than one or two services enabled here.

  8. Show all issues

    ~ before the class path?

  9. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    Localized?

  10. Show all issues

    Seeing a lot of inconsistency with the class paths in the docs. Some say reviewboard.accounts.avatars.services, some reviewboard.avatars.service, and some reviewboard.avatars.services.

  11. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
     
    Show all issues

    Might wrap better as:

    raise ItemLookupError(
        '...'
        % service)
    

    Same below.

    Also, these should be localized.

  12. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
     
    Show all issues

    I'm not sure it's clear what this really means, as written. Same below.

    Can we say what saving means, or maybe rewrite it if it's something like "Whether or not the list of enabled services should be saved to the database"? "Registry" isn't necessarily as clear.

  13. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    Sort of hitting a parse error with this sentence.

  14. Show all issues

    Not necessarily true. Maybe:

    Whether the registry of avatars was newly populated by this call. If ``False``, the registry was already populated by a previosu call.
    

    Also, do we use this anywhere? Might have missed it.

    1. If you want to override populate and call the super method, you have to check super(type(self), self).populate() to see if it is already populated; if it is, then we can exit early. See line 218.

  15. Show all issues

    We know we're saving below, so probably no need to do that here.

  16. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
     
    Show all issues

    Should pass the service ID as a parameter, rather than a format string.

  17. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
    Show all issues

    "set"/"unsetting" doesn't mean a lot outside of the internals of the registries, I don't think. Maybe we can reword this?

  18. reviewboard/accounts/avatars/registry.py (Diff revision 2)
     
     
     
     
     
    Show all issues

    This is an internal detail I'd really rather not expose. It's cheap to fetch the SiteConfiguration, since we cache it, so let's just do that here.

  19. Show all issues

    Since this is something that developers will be interacting with, this should go into a lot more detail on what this is responsible for, how it's used, and what's expected by an implementor.

  20. Show all issues

    This should go into more detail on what's expected and what's provided to the template.

  21. Show all issues

    How about "The user whose avatar URL will be retrieved."

  22. Show all issues

    Let's add "in pixels," and that this will be applied to both the width and the height.

  23. Show all issues

    "... to HTML."

    This should go into more detail about how it's rendered (using template), and that it can be overridden for custom behavior.

  24. reviewboard/accounts/avatars/services.py (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    Let's cache these on the request, taking into account the user ID and size. This will give us a nice speed boost. We should also mention that in the docs.

  25. Show all issues

    Let's have this in its own file. Refer to my thoughts at the top about directory structure.

  26. reviewboard/accounts/templatetags/avatars.py (Diff revision 2)
     
     
     
     
    Show all issues

    Two blank lines.

  27. Show all issues

    This is kind of oddly written, and doesn't make it clear that service_id can impact this. This should be more explicit.

  28. Show all issues

    Let's pass the values as keyword arguments, in case we extend down the road.

  29. 
      
chipx86
  1. Oh, also, I feel like this would be a great thing to have in Djblets, since avatars are a very generic concept.

  2. 
      
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/accounts/avatars/tests.py
        reviewboard/accounts/avatars/registry.py
        reviewboard/admin/siteconfig.py
        reviewboard/accounts/avatars/services.py
        reviewboard/accounts/avatars/errors.py
        reviewboard/accounts/templatetags/avatars.py
    
    Ignored Files:
        reviewboard/accounts/avatars/__init__.py
        reviewboard/accounts/templatetags/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/accounts/avatars/tests.py
        reviewboard/accounts/avatars/registry.py
        reviewboard/admin/siteconfig.py
        reviewboard/accounts/avatars/services.py
        reviewboard/accounts/avatars/errors.py
        reviewboard/accounts/templatetags/avatars.py
    
    Ignored Files:
        reviewboard/accounts/avatars/__init__.py
        reviewboard/accounts/templatetags/__init__.py
    
    
  2. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  3. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 25
     E241 multiple spaces after ':'
    
  4. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  5. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  6. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 19
     E241 multiple spaces after ':'
    
  7. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  8. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  9. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  10. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  11. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  12. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 29
     E241 multiple spaces after ':'
    
  13. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  14. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  15. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  16. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  17. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 32
     E241 multiple spaces after ':'
    
  18. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  19. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  20. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  21. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 17
     E241 multiple spaces after ':'
    
  22. reviewboard/admin/siteconfig.py (Diff revision 2)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  23. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
  2. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  3. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 39
     E241 multiple spaces after ':'
    
  4. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 32
     E241 multiple spaces after ':'
    
  5. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 25
     E241 multiple spaces after ':'
    
  6. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  7. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  8. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 19
     E241 multiple spaces after ':'
    
  9. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  10. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  11. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  12. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  13. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  14. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 29
     E241 multiple spaces after ':'
    
  15. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  16. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  17. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  18. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  19. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  20. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 17
     E241 multiple spaces after ':'
    
  21. reviewboard/admin/siteconfig.py (Diff revision 3)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  22. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'django_reset' imported but unused
    
  23. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  24. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'PIPELINE_CSS' imported but unused
    
  25. reviewboard/settings.py (Diff revision 3)
     
     
    Show all issues
     'PIPELINE_JS' imported but unused
    
  26. 
      
chipx86
  1. Typo in the description: "serach". That should probably just be its own trivial change, though.

  2. 
      
chipx86
  1. 
      
  2. reviewboard/avatars/__init__.py (Diff revision 3)
     
     
     
     
    Show all issues

    Two blank lines.

  3. reviewboard/avatars/registry.py (Diff revision 3)
     
     
     
    Show all issues

    This needs more detail on what this does and how it works, for people not familiar with what we're doing.

  4. reviewboard/avatars/registry.py (Diff revision 3)
     
     
    Show all issues

    This shouldn't have to be called here. It's already managed by the middleware and app initialize stuff, and when setting new configuration for the site. This should be able to rely on the existing calls.

  5. reviewboard/avatars/tests.py (Diff revision 3)
     
     
    Show all issues

    This makes me nervous. Can we just set the things we need?

  6. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
  2. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  3. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  4. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 24
     E241 multiple spaces after ':'
    
  5. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 25
     E241 multiple spaces after ':'
    
  6. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  7. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 23
     E241 multiple spaces after ':'
    
  8. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 19
     E241 multiple spaces after ':'
    
  9. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  10. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  11. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  12. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  13. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  14. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 29
     E241 multiple spaces after ':'
    
  15. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 30
     E241 multiple spaces after ':'
    
  16. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 31
     E241 multiple spaces after ':'
    
  17. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  18. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 26
     E241 multiple spaces after ':'
    
  19. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 28
     E241 multiple spaces after ':'
    
  20. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 17
     E241 multiple spaces after ':'
    
  21. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
    Show all issues
    Col: 22
     E241 multiple spaces after ':'
    
  22. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'django_reset' imported but unused
    
  23. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  24. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'PIPELINE_CSS' imported but unused
    
  25. reviewboard/settings.py (Diff revision 4)
     
     
    Show all issues
     'PIPELINE_JS' imported but unused
    
  26. 
      
david
  1. 
      
  2. reviewboard/admin/siteconfig.py (Diff revision 4)
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Things are misaligned here. Can we just shift this to be pep-8 style (even though it's not quite as pretty as aligned)?

  3. reviewboard/avatars/templatetags/avatars.py (Diff revision 4)
     
     
     
     
    Show all issues

    This should all fit on one line.

  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/admin/siteconfig.py
        reviewboard/avatars/__init__.py
        reviewboard/settings.py
        reviewboard/avatars/registry.py
        reviewboard/avatars/templatetags/avatars.py
        reviewboard/avatars/tests.py
    
    Ignored Files:
        reviewboard/avatars/templatetags/__init__.py
    
    
  2. reviewboard/settings.py (Diff revision 5)
     
     
    Show all issues
     'django_reset' imported but unused
    
  3. reviewboard/settings.py (Diff revision 5)
     
     
    Show all issues
     'from settings_local import *' used; unable to detect undefined names
    
  4. reviewboard/settings.py (Diff revision 5)
     
     
    Show all issues
     'PIPELINE_CSS' imported but unused
    
  5. reviewboard/settings.py (Diff revision 5)
     
     
    Show all issues
     'PIPELINE_JS' imported but unused
    
  6. 
      
david
  1. Ship It!
  2. 
      
brennie
brennie
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.6.x (4b9342b)