Add customizable avatar services

Review Request #8205 — Created June 1, 2016 and submitted

Information

Djblets
release-0.10.x
81259c3...

Reviewers

Djblets now supports avatar services that can be customized per-user via
a configuration form. It currently includes one such avatar service, the
FileUploadAvatarService, which allows users to upload an avatar of
their choosing.

The customization form uses nested configuration page forms to
dynamically show and hide the required fields, as well as include the
necessary CSS and JavaScript bundles for the forms.

Each avatar service that is customizable requires a SettingsManager,
which is set per-registry. The settings manager is responsible for
loading and saving the per-user configuration of each avatar service.
Djblets does not come with a default one (as it depends on the database
schema).

  • Ran unit tests.
  • Changed my avatar to an uploaded avatar in Review Board.
Description From Last Updated

'AvatarSettingsManager' imported but unused

reviewbotreviewbot

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

reviewbotreviewbot

Col: 1 W391 blank line at end of file

reviewbotreviewbot

Col: 1 W391 blank line at end of file

reviewbotreviewbot

Col: 1 W391 blank line at end of file

reviewbotreviewbot

Can you add a file docstring?

daviddavid

Can you add a file docstring?

daviddavid

Can you add a file docstring?

daviddavid

We repeat this loop in several places. Perhaps we should add an iterator to the registry that yields enabled services …

daviddavid

Do we want to mark this string for translation?

daviddavid

Can you add a file docstring?

daviddavid

Docstring?

daviddavid

These lines can be combined.

daviddavid

Can you add a file docstring?

daviddavid

I think for properties we want to write the docstring as if it were for regular data rather than a …

daviddavid

Same here.

daviddavid

Can remove this line.

daviddavid

This should be indented 4 more spaces.

daviddavid

Many of these class names don't look like they need to be nested. You can probably put each of the …

daviddavid

These can be combined. Perhaps also make this .avatar-missing for consistency, and put it at the top-level?

daviddavid

If we have dependencies right, this shouldn't be necessary.

daviddavid

Trailing whitespace.

daviddavid

Trailing whitespace.

daviddavid

Trailing whitespace.

daviddavid

More trailing whitespace.

daviddavid

And here...

daviddavid

I've used constructor here but either way callable is a python thing, and this should be, at a minimum, function.

daviddavid

Rather than have this method and the global readyPromise/ready, how about put the promise directly here as a property called …

daviddavid

Trailing whitespace.

daviddavid

This should probably happen inside of a render method.

daviddavid

no animated GIF avatars?

daviddavid

More here.

daviddavid

We can delay constructing the FileReader until inside the conditional.

daviddavid

This line can be removed.

daviddavid

This line can be removed.

daviddavid

Docstring?

daviddavid

Docstring?

daviddavid

Docstring?

daviddavid

Docstring?

daviddavid

Doc comment?

daviddavid

Trailing whitespace.

daviddavid

Trailing whitespace.

daviddavid

Col: 5 E303 too many blank lines (2)

reviewbotreviewbot

I don't understand why the text is inside a list. You don't do that for other ValidationErrors.

daviddavid

There's an extra line here.

daviddavid

I think we usually list classes before element types.

daviddavid

We should call Backbone.View.initialize here.

daviddavid

{} aren't needed in the fat arrow function. If it doesn't fit all on one line, put the fat arrow …

daviddavid

const works here?? I'm pretty sure that it needs to be let because it will assign multiple times as it …

daviddavid

Can we be more specific than just cls and type? Maybe something like formClass?

daviddavid

Add another blank line here.

daviddavid

Add another blank line here.

daviddavid

Add another blank line here.

daviddavid
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. djblets/avatars/pages.py (Diff revision 1)
     
     
    Show all issues
     'AvatarSettingsManager' imported but unused
    
  3. djblets/avatars/services/file_upload.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (82 > 79 characters)
    
  4. djblets/avatars/services/file_upload.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W391 blank line at end of file
    
  5. djblets/avatars/settings.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W391 blank line at end of file
    
  6. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. djblets/avatars/services/file_upload.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     W391 blank line at end of file
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/tests.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/configforms/templates/configforms/config.html
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
brennie
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
        djblets/avatars/pages.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
david
  1. 
      
  2. djblets/avatars/__init__.py (Diff revision 7)
     
     
    Show all issues

    Can you add a file docstring?

  3. djblets/avatars/apps.py (Diff revision 7)
     
     
    Show all issues

    Can you add a file docstring?

  4. djblets/avatars/forms.py (Diff revision 7)
     
     
    Show all issues

    Can you add a file docstring?

  5. djblets/avatars/forms.py (Diff revision 7)
     
     
     
    Show all issues

    We repeat this loop in several places. Perhaps we should add an iterator to the registry that yields enabled services that are configurable?

  6. djblets/avatars/forms.py (Diff revision 7)
     
     
    Show all issues

    Do we want to mark this string for translation?

  7. djblets/avatars/pages.py (Diff revision 7)
     
     
    Show all issues

    Can you add a file docstring?

  8. djblets/avatars/pages.py (Diff revision 7)
     
     
    Show all issues

    Docstring?

  9. djblets/avatars/services/file_upload.py (Diff revision 7)
     
     
     
    Show all issues

    These lines can be combined.

  10. djblets/avatars/settings.py (Diff revision 7)
     
     
    Show all issues

    Can you add a file docstring?

  11. djblets/avatars/settings.py (Diff revision 7)
     
     
    Show all issues

    I think for properties we want to write the docstring as if it were for regular data rather than a method. I'm not sure how adding a docstring on the setter affects things.

  12. djblets/avatars/settings.py (Diff revision 7)
     
     
    Show all issues

    Same here.

  13. Show all issues

    Can remove this line.

  14. Show all issues

    This should be indented 4 more spaces.

  15. djblets/static/djblets/css/avatars.less (Diff revision 7)
     
     
     
     
     
    Show all issues

    Many of these class names don't look like they need to be nested. You can probably put each of the .avatar-* ones at the top-level. As-is, you'll end up creating selectors like .avatar-service-configuration .avatar-service-fields .avatar-preview .missing-avatar::before, which isn't very efficient.

  16. djblets/static/djblets/css/avatars.less (Diff revision 7)
     
     
     
    Show all issues

    These can be combined. Perhaps also make this .avatar-missing for consistency, and put it at the top-level?

  17. Show all issues

    If we have dependencies right, this shouldn't be necessary.

  18. Show all issues

    Trailing whitespace.

  19. Show all issues

    Trailing whitespace.

  20. Show all issues

    Trailing whitespace.

  21. djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    More trailing whitespace.

  22. Show all issues

    And here...

  23. Show all issues

    I've used constructor here but either way callable is a python thing, and this should be, at a minimum, function.

  24. Show all issues

    Rather than have this method and the global readyPromise/ready, how about put the promise directly here as a property called ready? You can then set the resolver as something like:

    ready: new Promise((resolve, reject) => {
        Djblets.Avatars.SettingsFormView._readyResolver = resolve;
    }),
    
    1. Thanks! I was looking for a better way to do this.

  25. Show all issues

    Trailing whitespace.

  26. Show all issues

    This should probably happen inside of a render method.

  27. Show all issues

    no animated GIF avatars?

  28. Show all issues

    More here.

  29. Show all issues

    We can delay constructing the FileReader until inside the conditional.

  30. Show all issues

    This line can be removed.

  31. Show all issues

    This line can be removed.

  32. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
david
  1. 
      
  2. djblets/avatars/forms.py (Diff revision 8)
     
     
    Show all issues

    Docstring?

  3. djblets/avatars/forms.py (Diff revision 8)
     
     
    Show all issues

    Docstring?

  4. djblets/avatars/forms.py (Diff revision 8)
     
     
    Show all issues

    Docstring?

  5. djblets/avatars/forms.py (Diff revision 8)
     
     
    Show all issues

    Docstring?

  6. djblets/avatars/registry.py (Diff revision 8)
     
     
     
     
     
     
     
     
     
     
     
     
     

    This could be simplified a bit by returning a generator:

    self.populate()
    return (service for service in self.enabled_services
            if services.is_configurable)
    
  7. Show all issues

    Doc comment?

  8. Show all issues

    Trailing whitespace.

  9. Show all issues

    Trailing whitespace.

  10. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. djblets/avatars/registry.py (Diff revision 9)
     
     
    Show all issues
    Col: 5
     E303 too many blank lines (2)
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
david
  1. 
      
  2. djblets/avatars/forms.py (Diff revision 10)
     
     
    Show all issues

    I don't understand why the text is inside a list. You don't do that for other ValidationErrors.

    1. Probably because its a typo :)

  3. djblets/avatars/services/base.py (Diff revision 10)
     
     
    Show all issues

    There's an extra line here.

  4. djblets/static/djblets/css/avatars.less (Diff revision 10)
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    I think we usually list classes before element types.

  5. Show all issues

    We should call Backbone.View.initialize here.

    1. Backbone.View.initialize is an empty function. We don't do this anywhere else.

  6. Show all issues

    {} aren't needed in the fat arrow function. If it doesn't fit all on one line, put the fat arrow completely on the next line.

  7. Show all issues

    const works here?? I'm pretty sure that it needs to be let because it will assign multiple times as it iterates.

    1. In each iteration it is a different binding for each iteration. It also transpiles down to the same ES5 as let.

  8. Show all issues

    Can we be more specific than just cls and type? Maybe something like formClass?

  9. Show all issues

    Add another blank line here.

  10. Show all issues

    Add another blank line here.

  11. Show all issues

    Add another blank line here.

  12. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/avatars/forms.py
        djblets/staticbundles.py
        djblets/avatars/services/file_upload.py
        djblets/avatars/services/base.py
        djblets/avatars/apps.py
        djblets/avatars/settings.py
        djblets/avatars/__init__.py
        djblets/avatars/registry.py
    
    Ignored Files:
        djblets/avatars/templates/avatars/settings_form.html
        djblets/static/djblets/js/utils/promise.es6.js
        djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/base.js
        djblets/avatars/templates/avatars/services/file_upload_form.html
        djblets/static/djblets/css/avatars.less
        djblets/avatars/templates/avatars/service_form.html
        djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js
        djblets/static/djblets/js/avatars/models/avatarSettingsModel.es6.js
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed
Status:
Completed
Change Summary:
Pushed to release-0.10.x (2df5102)