Summary

Add customizable avatar services

Review Request #8205 — Created June 1, 2016 and submitted June 17, 2016, 5:12 p.m.

Information

Owner

brennie

Repository

Djblets

Branch

release-0.10.x

Bugs

Depends On

~~8202~~

Blocks

8206

Commit

81259c3...

Reviewers

Groups

djblets

People

Description

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

Testing Done


Ran unit tests.
Changed my avatar to an uploaded avatar in Review Board.

Issues

Description	From	Last Updated
'AvatarSettingsManager' imported but unused	reviewbot	June 1, 2016, 7:02 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	June 1, 2016, 7:02 p.m.
Col: 1 W391 blank line at end of file	reviewbot	June 1, 2016, 7:02 p.m.
Col: 1 W391 blank line at end of file	reviewbot	June 1, 2016, 7:02 p.m.
Col: 1 W391 blank line at end of file	reviewbot	June 1, 2016, 7:45 p.m.
Can you add a file docstring?	david	June 6, 2016, 10:09 p.m.
Can you add a file docstring?	david	June 6, 2016, 10:09 p.m.
Can you add a file docstring?	david	June 6, 2016, 10:09 p.m.
We repeat this loop in several places. Perhaps we should add an iterator to the registry that yields enabled services …	david	June 6, 2016, 10:26 p.m.
Do we want to mark this string for translation?	david	June 6, 2016, 10:29 p.m.
Can you add a file docstring?	david	June 6, 2016, 10:30 p.m.
Docstring?	david	June 6, 2016, 10:30 p.m.
These lines can be combined.	david	June 6, 2016, 10:34 p.m.
Can you add a file docstring?	david	June 6, 2016, 10:36 p.m.
I think for properties we want to write the docstring as if it were for regular data rather than a …	david	June 6, 2016, 10:37 p.m.
Same here.	david	June 6, 2016, 10:37 p.m.
Can remove this line.	david	June 6, 2016, 10:37 p.m.
This should be indented 4 more spaces.	david	June 6, 2016, 10:37 p.m.
Many of these class names don't look like they need to be nested. You can probably put each of the …	david	June 6, 2016, 10:39 p.m.
These can be combined. Perhaps also make this .avatar-missing for consistency, and put it at the top-level?	david	June 6, 2016, 10:40 p.m.
If we have dependencies right, this shouldn't be necessary.	david	June 6, 2016, 10:40 p.m.
Trailing whitespace.	david	June 6, 2016, 10:40 p.m.
Trailing whitespace.	david	June 6, 2016, 10:41 p.m.
Trailing whitespace.	david	June 6, 2016, 10:41 p.m.
More trailing whitespace.	david	June 6, 2016, 10:42 p.m.
And here...	david	June 6, 2016, 10:42 p.m.
I've used constructor here but either way callable is a python thing, and this should be, at a minimum, function.	david	June 6, 2016, 10:42 p.m.
Rather than have this method and the global readyPromise/ready, how about put the promise directly here as a property called …	david	June 6, 2016, 10:48 p.m.
Trailing whitespace.	david	June 6, 2016, 10:49 p.m.
This should probably happen inside of a render method.	david	June 6, 2016, 10:50 p.m.
no animated GIF avatars?	david	June 6, 2016, 10:51 p.m.
More here.	david	June 6, 2016, 10:51 p.m.
We can delay constructing the FileReader until inside the conditional.	david	June 6, 2016, 10:52 p.m.
This line can be removed.	david	June 6, 2016, 10:52 p.m.
This line can be removed.	david	June 6, 2016, 10:52 p.m.
Docstring?	david	June 8, 2016, 8:58 p.m.
Docstring?	david	June 8, 2016, 8:58 p.m.
Docstring?	david	June 8, 2016, 8:58 p.m.
Docstring?	david	June 8, 2016, 8:58 p.m.
Doc comment?	david	June 8, 2016, 9:01 p.m.
Trailing whitespace.	david	June 8, 2016, 9 p.m.
Trailing whitespace.	david	June 8, 2016, 9 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	June 8, 2016, 9:05 p.m.
I don't understand why the text is inside a list. You don't do that for other ValidationErrors.	david	June 13, 2016, 11:52 p.m.
There's an extra line here.	david	June 13, 2016, 11:53 p.m.
I think we usually list classes before element types.	david	June 13, 2016, 11:53 p.m.
We should call Backbone.View.initialize here.	david	June 11, 2016, 6:31 a.m.
{} aren't needed in the fat arrow function. If it doesn't fit all on one line, put the fat arrow …	david	June 13, 2016, 11:54 p.m.
const works here?? I'm pretty sure that it needs to be let because it will assign multiple times as it …	david	June 11, 2016, 6:31 a.m.
Can we be more specific than just cls and type? Maybe something like formClass?	david	June 13, 2016, 11:55 p.m.
Add another blank line here.	david	June 13, 2016, 11:55 p.m.
Add another blank line here.	david	June 13, 2016, 11:55 p.m.
Add another blank line here.	david	June 13, 2016, 11:55 p.m.

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

djblets/avatars/pages.py (Diff revision 1)
The issue has been resolved. Show all issues
```
 'AvatarSettingsManager' imported but unused
```
djblets/avatars/services/file_upload.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
djblets/avatars/services/file_upload.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W391 blank line at end of file
```
djblets/avatars/settings.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 1
 W391 blank line at end of file
```

Change Summary:

PEP8

Commit:

ae17acacc6c2617d64afdcbcc0f8f4984c7bd022

8c7cde518928dd79e0bdde9d9aba4e269c1bde44

Diff:

Revision 2 (+1016 -14)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/tests.py
	12 more

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

Change Summary:

Rename FileUploadAvatarService to FileUplaodService

Commit:

8c7cde518928dd79e0bdde9d9aba4e269c1bde44

058d061e724b0c4c103cf5a6bae2838e876b99c1

Diff:

Revision 3 (+1017 -14)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/tests.py
	12 more

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

djblets/avatars/services/file_upload.py (Diff revision 3)
The issue has been resolved. Show all issues
```
Col: 1
 W391 blank line at end of file
```

Change Summary:

PEP8

Commit:

058d061e724b0c4c103cf5a6bae2838e876b99c1

4388555603d07abd8569f48c227459db0925b7c7

Diff:

Revision 4 (+1023 -14)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/tests.py
	12 more

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

Depends On:: ~~8202 - Fix configuration forms not rendering JS config properly~~

Change Summary:

Rebase

Diff:

Revision 5 (+945 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	10 more

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

Change Summary:

rebase

Diff:

Revision 6 (+945 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	10 more

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

Commit:

4388555603d07abd8569f48c227459db0925b7c7

81259c3af5b3f430b52de82c19d9623ffcaf783d

Diff:

Revision 7 (+941 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/pages.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	10 more

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

djblets/avatars/__init__.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you add a file docstring?
```
djblets/avatars/apps.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you add a file docstring?
```
djblets/avatars/forms.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you add a file docstring?
```

djblets/avatars/forms.py (Diff revision 7)

The issue has been resolved. 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?

djblets/avatars/forms.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Do we want to mark this string for translation?
```
djblets/avatars/pages.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you add a file docstring?
```
djblets/avatars/pages.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Docstring?
```
djblets/avatars/services/file_upload.py (Diff revision 7)
The issue has been resolved. Show all issues
```
These lines can be combined.
```
djblets/avatars/settings.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Can you add a file docstring?
```

djblets/avatars/settings.py (Diff revision 7)

The issue has been resolved. 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.

djblets/avatars/settings.py (Diff revision 7)
The issue has been resolved. Show all issues
```
Same here.
```
djblets/avatars/templates/avatars/services/file_upload_form.html (Diff revision 7)
The issue has been resolved. Show all issues
```
Can remove this line.
```
djblets/avatars/templates/avatars/settings_form.html (Diff revision 7)
The issue has been resolved. Show all issues
```
This should be indented 4 more spaces.
```

djblets/static/djblets/css/avatars.less (Diff revision 7)

The issue has been resolved. 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.

djblets/static/djblets/css/avatars.less (Diff revision 7)

The issue has been resolved. Show all issues

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

djblets/static/djblets/js/avatars/base.js (Diff revision 7)
The issue has been resolved. Show all issues
```
If we have dependencies right, this shouldn't be necessary.
```
djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
djblets/static/djblets/js/avatars/views/avatarServiceSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)

The issue has been resolved. Show all issues

More trailing whitespace.

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
And here...
```
djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
I've used constructor here but either way callable is a python thing, and this should be, at a minimum, function.
```

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 7)

The issue has been resolved. 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;
}),

brennie June 4, 2016, 4:59 a.m.

Thanks! I was looking for a better way to do this.

djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
This should probably happen inside of a render method.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
no animated GIF avatars?
```

djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)

The issue has been resolved. Show all issues

More here.

djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
We can delay constructing the FileReader until inside the conditional.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
This line can be removed.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 7)
The issue has been resolved. Show all issues
```
This line can be removed.
```

Diff:

Revision 8 (+969 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	djblets/avatars/services/file_upload.py
	10 more

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

djblets/avatars/forms.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Docstring?
```
djblets/avatars/forms.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Docstring?
```
djblets/avatars/forms.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Docstring?
```
djblets/avatars/forms.py (Diff revision 8)
The issue has been resolved. Show all issues
```
Docstring?
```

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)

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 8)
The issue has been resolved. Show all issues
```
Doc comment?
```
djblets/static/djblets/js/utils/promise.es6.js (Diff revision 8)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```
djblets/static/djblets/js/utils/promise.es6.js (Diff revision 8)
The issue has been resolved. Show all issues
```
Trailing whitespace.
```

Change Summary:

Addressed issues.

Diff:

Revision 9 (+1016 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	djblets/avatars/services/file_upload.py
	10 more

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

djblets/avatars/registry.py (Diff revision 9)
The issue has been resolved. Show all issues
```
Col: 5
 E303 too many blank lines (2)
```

Change Summary:

pep8

Diff:

Revision 10 (+1015 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	djblets/avatars/services/file_upload.py
	10 more

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

djblets/avatars/forms.py (Diff revision 10)

The issue has been resolved. Show all issues

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

brennie June 11, 2016, 6:31 a.m.
```
Probably because its a typo :)
```

djblets/avatars/services/base.py (Diff revision 10)
The issue has been resolved. Show all issues
```
There's an extra line here.
```

djblets/static/djblets/css/avatars.less (Diff revision 10)

The issue has been resolved. Show all issues

I think we usually list classes before element types.

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 10)
The issue has been dropped. Show all issues
```
We should call Backbone.View.initialize here.
```
1. brennie June 11, 2016, 6:31 a.m.
  Backbone.View.initialize is an empty function. We don't do this anywhere else.

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 10)

The issue has been resolved. 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.

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 10)

The issue has been dropped. 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.

brennie June 11, 2016, 6:31 a.m.

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

djblets/static/djblets/js/avatars/views/avatarSettingsFormView.es6.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Can we be more specific than just cls and type? Maybe something like formClass?
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Add another blank line here.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Add another blank line here.
```
djblets/static/djblets/js/avatars/views/fileUploadSettingsFormView.es6.js (Diff revision 10)
The issue has been resolved. Show all issues
```
Add another blank line here.
```

Change Summary:

Addressed David's issues

Diff:

Revision 11 (+1016 -12)

Show changes

	djblets/staticbundles.py
	djblets/avatars/__init__.py
	djblets/avatars/apps.py
	djblets/avatars/forms.py
	djblets/avatars/registry.py
	djblets/avatars/settings.py
	djblets/avatars/services/base.py
	djblets/avatars/services/file_upload.py
	10 more

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

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-0.10.x (2df5102)