Hey Florie - did you try generating the sourcecode for the avatars to make sure that it's properly formatted? If so, you should add that to Testing Done. If not, I suggest doing that.
I think you can do that from within the djblets/docs/djblets directory, and running make html. You might have to install some packages (like sphinx).

The Bugs field needs to be filled in with the bugs this references.

The documentation is great, but I think part of the original problem in the report is that the user saw a crash and didn't know what caused it immediately. So, along with the documentation improvements, we should make sure things do not crash, but rather that raised exceptions are logged.
One place to do this is in djblets/avatars/base.py, where get_avatar_urls_uncached is called. We should catch the exception, log the error, and return a sane result (like {}).
The other function, get_etag_data is actually not called in Djblets. We probably should not be raising NotImplementedError here, since we can provide a sane default. So, I'd recommend altering the function to return a sane default, like:

return [self.avatar_service_id, user.pk]


This would probably be an okay default, and would prevent blowing up.
Unit tests need to be added to:

Test calling get_avatar_urls without get_avatar_urls_uncached having an implementation, and ensuring that we get a sane result back and that data is logged (you'll use kgb for this -- ask David or Barret about how to do this).
Test that get_etag_data returns that default value.

The description/testing need additional work to meet our standards. We have a guide on what we expect from these fields: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/

There needs to be a blank line separating the list and the text preceding it.

No "and" at the end here. For lists of items in docs, you don't treat each item as part of a sentence.

No need for "attributes".

Given the above, you'd want to rewrite this as well.

No need for "method".
There needs to be a blank line separating the list and the text preceding it.

This should go before the django imports.
We put imports into three groups:


from __future__ import unicode_literals #always first

import logging # Python standard library
import os

from django.conf import foo # 3-rd Party imports

from djblets.foo import bar # Project imports

Missing a period. Should mention that they should override get_etag_data for caching.

Undo this change. We want this to raise an error.

Can you also change this into a definition list?

This should use a definition list:

Subclasses must also override:

:py:meth:`~base.AvatarService.get_avatar_urls_uncached`:
    This method computes and returns the avatar URLs for the given user at a requested size at different resolutions

:py:meth:`~base.AvatarService.get_etag_data`:
     This method returns a list of unicode strings that uniquely identify the avatar services and its configuration.

We should mention get_etag_data has a default impl but it should probably be overriden to include appropriate setting values for caching.

Can you update the summary to indicate this is a docs change? Something like:

Update AvatarService docs and guide to include required fields and methods

Making some small changes and landing. Thanks!