Refactor notifications.email into a module

Review Request #8829 - Created March 20, 2017 and submitted

Barret Rennie
Review Board
release-3.0.x
8889
72646df...
reviewboard

email.py was getting long and it is past due that it was split up.
This patch is in preparation for another change that will add more
e-mail-specific code.

Ran unit tests.

  • 0
  • 0
  • 21
  • 3
  • 24
Description From Last Updated
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/tests.py
        reviewboard/notifications/email/callbacks.py
        reviewboard/notifications/email/__init__.py
        reviewboard/notifications/email/message.py
        reviewboard/accounts/forms/pages.py
        reviewboard/notifications/email/utils.py
        reviewboard/notifications/email/hooks.py
        reviewboard/notifications/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/tests.py
        reviewboard/notifications/email/callbacks.py
        reviewboard/notifications/email/__init__.py
        reviewboard/notifications/email/message.py
        reviewboard/accounts/forms/pages.py
        reviewboard/notifications/email/utils.py
        reviewboard/notifications/email/hooks.py
        reviewboard/notifications/__init__.py
    
    
  2.  'register_email_hook' imported but unused
    
  3.  'unregister_email_hook' imported but unused
    
  4.  undefined name 'logging'
    
  5. 
      
Barret Rennie
Barret Rennie
Review Bot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/notifications/tests.py
        reviewboard/notifications/email/callbacks.py
        reviewboard/notifications/email/__init__.py
        reviewboard/notifications/email/message.py
        reviewboard/accounts/forms/pages.py
        reviewboard/notifications/email/utils.py
        reviewboard/notifications/email/hooks.py
        reviewboard/notifications/__init__.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/notifications/tests.py
        reviewboard/notifications/email/callbacks.py
        reviewboard/notifications/email/__init__.py
        reviewboard/notifications/email/message.py
        reviewboard/accounts/forms/pages.py
        reviewboard/notifications/email/utils.py
        reviewboard/notifications/email/hooks.py
        reviewboard/notifications/__init__.py
    
    
  2. 
      
David Trowbridge
  1. 
      
  2. reviewboard/notifications/__init__.py (Diff revision 2)
     
     

    I think connect_signals should still live in the top-level email module.

  3. Blank line after this.

  4. File docstring?

  5. Blank line after this?

  6. Blank line after this?

  7. Blank line after this?

  8. 
      
Barret Rennie
Christian Hammond
  1. 
      
  2. These modules need to be listed in the codebase docs.

  3. reviewboard/notifications/email/callbacks.py (Diff revision 3)
     
     
     
     

    There's 3 blank lines here.

  4. reviewboard/notifications/email/__init__.py (Diff revision 3)
     
     
     
     

    No blank line.

  5. reviewboard/notifications/email/__init__.py (Diff revision 3)
     
     
     
     
     

    We usually list this last in the file.

    This doesn't contain connect_signals.

    This is also most commonly a list (all Python docs, PEP8, etc. all show as a list instead of a tuple). The rest of our code uses a list as well, so let's change this one.

    1. I think people are slowly migrating towards using tuples because they're immutable.

  6. reviewboard/notifications/email/__init__.py (Diff revision 3)
     
     
     
     
     
     

    Two blank lines.

  7. "Extension hooks for augmenting e-mails" will be more useful in the codebase docs.

  8. reviewboard/notifications/email/hooks.py (Diff revision 3)
     
     
     
     

    Should be two blank lines.

  9. This loses the information we had in the original docstring.

  10. 
      
Barret Rennie
Christian Hammond
  1. Some of this is from code that was already there, but while we're splitting into a new module...

  2. docs/manual/extending/coderef/index.rst (Diff revision 4)
     
     

    I guess we can skip this one, since it's purely internal callbacks.

  3. reviewboard/notifications/email/callbacks.py (Diff revision 4)
     
     
     
     
     
     

    Too many blank lines.

  4. reviewboard/notifications/email/__init__.py (Diff revision 4)
     
     
     

    Inconsistent alignment.

  5. Kinda wonder if this should just live in callbacks.py. Maybe rename that to something like signal_handlers.py and have the base notifications/__init__.py import that directly? What do you think?

    1. That makes the import of connect_signals from each module (webhooks, email) less symmetric. I think this is good as is, but Im cool with the rename.

  6. tuple should be on its own line.

  7. tuple should be on its own line.

  8. reviewboard/notifications/email/utils.py (Diff revision 4)
     
     
     

    list should be on its own line.

  9. 
      
Barret Rennie
Christian Hammond
  1. Ship It!
  2. 
      
Barret Rennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-3.0.x (468c1a6)
Loading...