Add registries, which can be used to keep track of unique objects

Review Request #7780 — Created Nov. 25, 2015 and submitted

Information

Djblets
release-0.10.x

Reviewers

Registries are wrappers around Python's set class that provides the
additional constraints:
1. All registered members must have a core set of attributes
(defined per registry).
2. The values for these attributes must be unique across all members.

A guide on writing registries has been added to the documentation.

  • Ran unit tests.
  • Built and viewed the docs.
Description From Last Updated

'MissingAttributesError' imported but unused

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

This isn't the right form for this. You need: __all__ = ['Registry']

chipx86chipx86

Could this be NotRegisteredError?

chipx86chipx86

How about AlreadyRegisteredError? DuplicateItemError is far too generic.

chipx86chipx86

I feel like this is too confusing to a consumer of this module. I have no idea what it means …

chipx86chipx86

"Any" isn't really valid for these, I don't think? Let's use object.

chipx86chipx86

'Registry' imported but unused

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

Col: 50 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 50 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 50 E128 continuation line under-indented for visual indent

reviewbotreviewbot

'Registry' imported but unused

reviewbotreviewbot

This still has the wrong casing for __all__.

chipx86chipx86

Every new file should have a file docstring, so that it's better covered in the module index. It could point …

chipx86chipx86

Two blank lines between these.

chipx86chipx86

These should be localized. While doing that, I'd suggest this formatting: DEFAULT_ERRORS = { 'key': _( 'string', ), ... } …

chipx86chipx86

Needs a docstring. Also, does this need to be a class method?

chipx86chipx86

I don't know if this is the case, but if this isn't present in a subclass that overrides errors, do …

chipx86chipx86

This makes it sound like this requires the items up-front. We should be clear that this is for optionally pre-registering …

chipx86chipx86

Missing a period.

chipx86chipx86

Let's fetch this from _registry only once (we do it again below).

chipx86chipx86

Let's call this unregister_by_attr, so it's explicit. (We may decide we want to unregister by some other thing down the …

chipx86chipx86

Subclasses shouldn't override private methods. I think we need to change the naming of this and populate. What I'd suggest, …

chipx86chipx86

Do we want this to be public? I'm not sure it's needed, since everything else invokes it anyway.

chipx86chipx86

Since _items is a set, this won't provide any sort of stable ordering. Assuming that's okay (probably is), we need …

chipx86chipx86

testing should go last.

chipx86chipx86

Missing period.

chipx86chipx86

Two blank lines.

chipx86chipx86

I don't know that we need to say that they're based on set. That's an implementation detail. This should focus …

chipx86chipx86

"subclassed".

chipx86chipx86

Wrapping is weird.

chipx86chipx86

I don't like having this be stringly typed. Can we make an enum for the error types?

daviddavid

Reprs typically have <> around them: return '<Item: %s>' % ...

daviddavid

Should be "raising" instead of "returning"

daviddavid

Add another blank line here.

daviddavid

Should use ALREADY_REGISTERED.

daviddavid

This isn't wrapped right.

daviddavid

Too many blank lines here.

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
  2. djblets/util/registry/tests.py (Diff revision 1)
     
     
     'MissingAttributesError' imported but unused
    
  3. djblets/util/registry/tests.py (Diff revision 1)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/util/registry/__init__.py
        djblets/util/registry/tests.py
        djblets/util/registry/errors.py
        djblets/util/registry/registry.py
    
    
  2. 
      
chipx86
  1. I'll read over this in more detail later (probably when I'm back into town), but I have some general comments up-front:

    1. I want to avoid putting new things into djblets.util, since that's how it ends up just growing over time. Certainly there should be no new modules that have submodules in there. We did a big move to split out much of that into their own apps, and this is something I'd much rather see as its own djblets.registries.
    2. Now that we have codebase docs for Djblets, and are gearing toward making it easier for developers to write code against that and against our extension stuff, I'd very much like to have any new module like this have an accompanying guide or two for writing/using registries in djblets/docs/djblets/guides/registry/.
    3. The exceptions and terminology used here confuse me, and I think it's too heavy on "items" and "attributes." It makes them sound like dictionaries, but that's not how a consumer of this will think. I'd like the terminology, errors, and docs to be a lot more clear, more obvious.
    4. Along those lines, error messages should be based on the registry. Talking about "the item is already registered" is too vague and a major step down from what we have. Instead, they should be based on the thing that the registry is dealing with. '"%s" is already a registered hosting service' is a perfectly fine error, and we should strive to keep that sort of thing.
  2. djblets/util/registry/__init__.py (Diff revision 3)
     
     

    This isn't the right form for this. You need:

    __all__ = ['Registry']
    
  3. djblets/util/registry/errors.py (Diff revision 3)
     
     
     

    Could this be NotRegisteredError?

    1. Thats not really what this is.

      This is the case for when you have a registry for e.g. the id attribute, but try to look up something by name, etc.

  4. djblets/util/registry/errors.py (Diff revision 3)
     
     

    How about AlreadyRegisteredError? DuplicateItemError is far too generic.

  5. djblets/util/registry/errors.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    I feel like this is too confusing to a consumer of this module. I have no idea what it means unless I jump in and look at the implementation. Doesn't a duplicate attribute mean that it's already been registered, and if so, can we not just use the prior exception?

    I actually don't understand any of the exceptions in here aside from DuplicateItemError.

  6. djblets/util/registry/registry.py (Diff revision 3)
     
     

    "Any" isn't really valid for these, I don't think? Let's use object.

  7. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 4)
     
     
     'Registry' imported but unused
    
  3. djblets/registries/tests.py (Diff revision 4)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  4. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 5)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 6)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 7)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 8)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 9)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 10)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 11)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 12)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 13)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 14)
     
     
     'Registry' imported but unused
    
  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 15)
     
     
     'Registry' imported but unused
    
  3. djblets/registries/registry.py (Diff revision 15)
     
     
    Col: 50
     E128 continuation line under-indented for visual indent
    
  4. djblets/registries/registry.py (Diff revision 15)
     
     
    Col: 50
     E128 continuation line under-indented for visual indent
    
  5. djblets/registries/registry.py (Diff revision 15)
     
     
    Col: 50
     E128 continuation line under-indented for visual indent
    
  6. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. djblets/registries/__init__.py (Diff revision 16)
     
     
     'Registry' imported but unused
    
  3. 
      
chipx86
  1. I'll give this a more thorough review later (working on student reviews), but can you update the summary to be more self-descriptive? Like, "Add a foundation for implementing object registry APIs" or something?

  2. 
      
brennie
chipx86
  1. 
      
  2. djblets/registries/__init__.py (Diff revision 16)
     
     

    This still has the wrong casing for __all__.

  3. djblets/registries/registry.py (Diff revision 16)
     
     

    Every new file should have a file docstring, so that it's better covered in the module index. It could point to the guide as well.

  4. djblets/registries/registry.py (Diff revision 16)
     
     
     
     

    Two blank lines between these.

  5. djblets/registries/registry.py (Diff revision 16)
     
     
     
     
     
     
     
     
     
     
     
     
     

    These should be localized. While doing that, I'd suggest this formatting:

    DEFAULT_ERRORS = {
        'key': _(
            'string',
         ),
         ...
    }
    

    That way, it's easier to scan and update, and you get consistent alignment.

  6. djblets/registries/registry.py (Diff revision 16)
     
     

    Needs a docstring.

    Also, does this need to be a class method?

  7. djblets/registries/registry.py (Diff revision 16)
     
     

    I don't know if this is the case, but if this isn't present in a subclass that overrides errors, do we hit this assert? If so, we need a useful exception for that.

    1. It looks up the error in self.errors followed by self._default_errors, so we should never hit this for a valid error name.

  8. djblets/registries/registry.py (Diff revision 16)
     
     

    This makes it sound like this requires the items up-front. We should be clear that this is for optionally pre-registering items.

    I kind of think that this should take an explicit list, though, rather than using *args. That limits our ability to update it down the road.

  9. djblets/registries/registry.py (Diff revision 16)
     
     

    Missing a period.

  10. djblets/registries/registry.py (Diff revision 16)
     
     

    Let's fetch this from _registry only once (we do it again below).

  11. djblets/registries/registry.py (Diff revision 16)
     
     

    Let's call this unregister_by_attr, so it's explicit. (We may decide we want to unregister by some other thing down the road, like maybe an item's class, or something, or a subclass may want a version of this.)

  12. djblets/registries/registry.py (Diff revision 16)
     
     
     
     
     
     
     
     

    Subclasses shouldn't override private methods. I think we need to change the naming of this and populate.

    What I'd suggest, actually, is having a get_default_items method that returns a value, and have populate handle it below.

  13. djblets/registries/registry.py (Diff revision 16)
     
     

    Do we want this to be public? I'm not sure it's needed, since everything else invokes it anyway.

    1. Theres not harm in having a method to ensure its populated.

  14. djblets/registries/registry.py (Diff revision 16)
     
     
     
     
     
     
     
     
     
     
     

    Since _items is a set, this won't provide any sort of stable ordering. Assuming that's okay (probably is), we need to document that.

  15. djblets/registries/tests.py (Diff revision 16)
     
     
     
     
     

    testing should go last.

  16. djblets/registries/tests.py (Diff revision 16)
     
     

    Missing period.

  17. Two blank lines.

  18. I don't know that we need to say that they're based on set. That's an implementation detail. This should focus entirely on what they're for, how they're used.

    Overall, this guide should focus more about walking someone through their purpose and usage. An example and a couple attribute guides are useful, but reading through this, without knowing what registries are for, I wouldn't know what registries are for.

    Some high-level examples of their usage (conceptual, with an example for each) would be good. Basically, mini sections on "Here's how you'd do XYZ."

  19. Wrapping is weird.

  20. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
        djblets/registries/__init__.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        docs/djblets/coderef/index.rst
    
    
  2. 
      
david
  1. 
      
  2. djblets/registries/registry.py (Diff revision 18)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    I don't like having this be stringly typed. Can we make an enum for the error types?

  3. djblets/registries/tests.py (Diff revision 18)
     
     

    Reprs typically have <> around them:

    return '<Item: %s>' % ...

  4. Should be "raising" instead of "returning"

  5. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        djblets/registries/__init__.py
        docs/djblets/coderef/index.rst
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        djblets/registries/__init__.py
        docs/djblets/coderef/index.rst
    
    
  2. 
      
david
  1. 
      
  2. djblets/registries/registry.py (Diff revision 19)
     
     

    Add another blank line here.

  3. djblets/registries/registry.py (Diff revision 19)
     
     

    Should use ALREADY_REGISTERED.

  4. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        djblets/registries/__init__.py
        docs/djblets/coderef/index.rst
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/registries/errors.py
        djblets/registries/tests.py
        djblets/registries/registry.py
    
    Ignored Files:
        docs/djblets/guides/index.rst
        docs/djblets/guides/registries/index.rst
        docs/djblets/guides/registries/writing-registries.rst
        djblets/registries/__init__.py
        docs/djblets/coderef/index.rst
    
    
  2. 
      
david
  1. Just a couple trivial issues in the docs and then this looks good to me.

  2. This isn't wrapped right.

  3. docs/djblets/guides/registries/writing-registries.rst (Diff revision 20)
     
     
     
     
     

    Too many blank lines here.

  4. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.10.x (5f282bc)
Loading...