-
-
reviewboard/notifications/email/__init__.py (Diff revision 1) 'register_email_hook' imported but unused
-
reviewboard/notifications/email/__init__.py (Diff revision 1) 'unregister_email_hook' imported but unused
-
Refactor notifications.email into a module
Review Request #8829 — Created March 20, 2017 and submitted
Information | |
---|---|
brennie | |
Review Board | |
release-3.0.x | |
8889 | |
72646df... | |
Reviewers | |
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.
Description | From | Last Updated |
---|---|---|
These modules need to be listed in the codebase docs. |
|
|
'register_email_hook' imported but unused |
![]() |
|
'unregister_email_hook' imported but unused |
![]() |
|
undefined name 'logging' |
![]() |
|
I think connect_signals should still live in the top-level email module. |
|
|
Blank line after this. |
|
|
File docstring? |
|
|
Blank line after this? |
|
|
Blank line after this? |
|
|
Blank line after this? |
|
|
There's 3 blank lines here. |
|
|
No blank line. |
|
|
We usually list this last in the file. This doesn't contain connect_signals. This is also most commonly a list (all … |
|
|
Two blank lines. |
|
|
"Extension hooks for augmenting e-mails" will be more useful in the codebase docs. |
|
|
Should be two blank lines. |
|
|
This loses the information we had in the original docstring. |
|
|
I guess we can skip this one, since it's purely internal callbacks. |
|
|
Too many blank lines. |
|
|
Inconsistent alignment. |
|
|
Kinda wonder if this should just live in callbacks.py. Maybe rename that to something like signal_handlers.py and have the base … |
|
|
tuple should be on its own line. |
|
|
tuple should be on its own line. |
|
|
list should be on its own line. |
|

Change Summary:
Reflow description
Description: |
|
---|
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+425 -388) |

-
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
-
-
reviewboard/notifications/__init__.py (Diff revision 2) I think
connect_signals
should still live in the top-levelemail
module. -
-
-
-
-
Change Summary:
Addressed David's comments.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+450 -393) |
Checks run (2 succeeded, 1 failed with error)
-
-
-
-
-
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.
-
-
reviewboard/notifications/email/hooks.py (Diff revision 3) "Extension hooks for augmenting e-mails" will be more useful in the codebase docs.
-
-
reviewboard/notifications/email/message.py (Diff revision 3) This loses the information we had in the original docstring.
Change Summary:
Addressed Christian's issues.
Checks run (2 succeeded, 1 failed with error)
-
Some of this is from code that was already there, but while we're splitting into a new module...
-
docs/manual/extending/coderef/index.rst (Diff revision 4) I guess we can skip this one, since it's purely internal callbacks.
-
-
-
reviewboard/notifications/email/__init__.py (Diff revision 4) Kinda wonder if this should just live in
callbacks.py
. Maybe rename that to something likesignal_handlers.py
and have the basenotifications/__init__.py
import that directly? What do you think? -
-
-
Diff: |
Revision 5 (+464 -394)
|
---|