Expand on X-ReviewBoard-ShipIt by adding X-ReviewBoard-Conversation and X-ReviewClosed for commonly filtered e-mails.
Review Request #7803 — Created Dec. 10, 2015 and discarded
On our own team, we have found X-ReviewBoard-ShipIt very useful for picking out e-mails that contain actionable content (i.e. [Ship It]). In our group, the next most actionable content is "someone has commented on code / someone has responded to comments"
Now that we have X-ReviewBoard-ShipIt and X-ReviewBoard-ShipIt-Only to help filter [Ship It] messages, I figure it makes sense to add something like X-ReviewBoard-Conversation for those who want to filter specifically for messages containing review responses (or other conversation).
If there were something like X-ReviewBoard-Conversation or X-ReviewBoard-Comments to mark when a conversation is taking place (as opposed to just an updated diff/description, etc.) that would cover all of our most common e-mail filter requests.
Added a unit test for X-ReviewBoard-Conversation
If anyone has tips on the best way to test review open/close or replies that would be greatly appreciated. I'd love to unit test those too (although this code is running live-patched on our team's server full-time and works fine for us)
| Description | From | Last Updated |
|---|---|---|
|
The way that X-ReviewBoard-ShipIt-Only works is that the review request must meet the following conditions: There are no diff / … |
 brennie
|
|
|
With my above comments, I feel like there should be 2 cases: 1 is conversation and one is not-conversation. This … |
brennie
|
|
|
Col: 80 E501 line too long (119 > 79 characters) |
 reviewbot
|
|
|
We wrap long test docstrings as: """Test test test test test test test test test """ |
brennie
|
|
|
Missing a test for review replies. |
brennie
|
- Bugs:
-
-
-
The way that
X-ReviewBoard-ShipIt-Onlyworks is that the review request must meet the following conditions:- There are no diff / attachment comments.
- The text of the review is either "Ship It!" or empty.
- The review has a ship it.
I feel like it would be inappropriate to have
X-ReviewBoard-Conversationfor all review emails, because a conversation email should never be a shipit-only email.Thoughts?
-
With my above comments, I feel like there should be 2 cases: 1 is conversation and one is not-conversation. This is open to debate.
-
-
Tool: Pyflakes Processed Files: reviewboard/notifications/email.py reviewboard/notifications/tests.py Tool: PEP8 Style Checker Processed Files: reviewboard/notifications/email.py reviewboard/notifications/tests.py