Summary

Include branch field in review request emails

Review Request #10147 — Created Sept. 16, 2018 and submitted Oct. 24, 2018, 1:56 p.m.

Information

Owner

bolariinwa

Repository

Review Board

Branch

release-3.0.x

Bugs

4707

Depends On

Commit

f21537d...

Reviewers

Groups

reviewboard, students

People

Description

With a lot of review request emails, prioritizing review requests emails
becomes uneasy.

Review request e-mails will now contain a branch field.

Testing Done

Tested by manually checking generated review request emails for the
inclusion of the branch field.

Files

Issues

Description	From	Last Updated
Can you wrap your description at 72 characters instead of 100+? Same for testing done.	brennie	Sept. 21, 2018, 7:23 a.m.
Can you add a screenshot of the rendered HTML e-mail view with the new content?	brennie	Sept. 21, 2018, 7:23 a.m.
I know this is a small change, but can you take a peek at our guide for writing good change …	brennie	Sept. 22, 2018, 12:23 p.m.
It's also not quite right here. Probably need to include a <br>.	david	Oct. 18, 2018, 6 p.m.
This doesn't look right. What we want is: Repository: Review Board Branch: release-3.0.x	david	Oct. 18, 2018, 6 p.m.
We probably want these one right after the other. Unfortunately, the Django templating language is kinda messy when it comes …	chipx86	Sept. 25, 2018, 6:43 p.m.
Indentation here is wrong. We do our {% blocktag %} indentation inside the tags themselves and do not increase the …	brennie	Oct. 2, 2018, 4:10 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Oct. 18, 2018, 6:32 p.m.
E722 do not use bare except'	reviewbot	Oct. 18, 2018, 6:32 p.m.

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues

Can you wrap your description at 72 characters instead of 100+?

Same for testing done.

The issue has been resolved. Show all issues

Can you add a screenshot of the rendered HTML e-mail view with the new content?

Change Summary:: Add screenshot of newly rendered HTML and text email view with the new content
Added Files:

Change Summary:: Reworded description and testing to keep character count at 72
Description:: ~
With a lot of review request emails, it would be easier to prioritize requests if the email contained the branch
~
Inclusion of branch in review request emails to aid request prioritization
- that the change occured on.
-
-
This change includes the branch name in review request emails.
Testing Done:: ~
Testing was done manually. I checked for the branch name in review request emails after the change was made.
~
Newly generated views were checked for the inclusion of the branch field
- All review request emails contained the branch name.

The issue has been resolved. Show all issues

I know this is a small change, but can you take a peek at our guide for writing good change descriptions. Specifically, the description and testing done should consist of complete sentences.

Also the description should read more like

E-mails will now contain the contents of the branch field ....

i.e., the sentences should be descriptive not summaritive.

Change Summary:: Improved wording of description and testing
Description:: ~
Inclusion of branch in review request emails to aid request prioritization
~
With a lot of review request emails, prioritizing review requests emails
+ becomes uneasy.
+
+
Review request e-mails will now contain a branch field.
Testing Done:: ~
Newly generated views were checked for the inclusion of the branch field
~
Tested by manually checking generated review request emails for the
+ inclusion of the branch field.

reviewboard/templates/notifications/review_request_email.txt (Diff revision 1)

The issue has been resolved. Show all issues

We probably want these one right after the other. Unfortunately, the Django templating language is kinda messy when it comes to newlines (since it's really built for HTML where it rarely matters). Each newline in there, including the ones after the template tags, will be included in the plain-text e-mail. That means you're going to get something like:

Repository: my-repo



Branch: my-branch


Description
-----------

(This is only a problem in plain-text e-mails.)

Instead, we'll want to collapse those. The good news is, we have a handy tag you can use. called condense. This would look like:

{% condense 0 %}
{%  if review_request.repository %}
Repository: ...
{%  endif %}
{%  if review_request.branch %}
Branch: ...
{%  endif %}
{% endcondense %}

Description.....

Make sure you run the preview-email view with this and check the output to see that there's consistency in spacing between each section and each important thing within it. We'll want to see that in the testing.

Commit:

f327f7f81d60ca70b2ca8f5afd8582240a65c541

1350456630384cbf567a43f0551c73eca9434417

Diff:

Revision 2 (+14 -3)

Show changes

	reviewboard/templates/notifications/review_request_email.html
	reviewboard/templates/notifications/review_request_email.txt

Added Files:

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/templates/notifications/review_request_email.html (Diff revision 2)

The issue has been resolved. Show all issues

Indentation here is wrong. We do our {% blocktag %} indentation inside the tags themselves and do not increase the indentation of nest tags.
This should be formatted as:

<div style="margin-top: 1.5em">
{%   if review_request.repository %}
 <strong style="....">Repository: </strong>
 {{review_request.repository.name}}
{%   endif %}
{%   if review_request.branch %}
 <strong style="...">Branch: </strong>
 {{review_request.branch}}
{%   endif %}
</div>

Commit:

1350456630384cbf567a43f0551c73eca9434417

25652f9bb18e6a2fefa3993e3c03764d22a4d73c

Diff:

Revision 3 (+13 -2)

Show changes

	reviewboard/templates/notifications/review_request_email.html
	reviewboard/templates/notifications/review_request_email.txt

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Screenshot of HTML.png (Revision 1)
The issue has been resolved. Show all issues
```
It's also not quite right here. Probably need to include a <br>.
```

Screenshot of txt file.png (Revision 1)

The issue has been resolved. Show all issues

This doesn't look right. What we want is:

Repository: Review Board
Branch: release-3.0.x

Change Summary:

Addressed issues

Commit:

25652f9bb18e6a2fefa3993e3c03764d22a4d73c

7942f7203bfd549abc3e5d51badaf2d073b9e270

Diff:

Revision 4 (+136 -1042)

Show changes

	AUTHORS
	doc-requirements.txt
	contrib/internal/prepare-dev.py
	docs/manual/admin/installation/creating-sites.rst
	docs/releasenotes/3.0.9.rst
	docs/releasenotes/index.rst
	reviewboard/__init__.py
	reviewboard/dependencies.py
	18 more

Added Files:

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

contrib/internal/prepare-dev.py (Diff revision 4)
The issue has been resolved. Show all issues
```
E501 line too long (80 > 79 characters)
```
reviewboard/webapi/resources/review_request_draft.py (Diff revision 4)
The issue has been resolved. Show all issues
```
E722 do not use bare except'
```

Change Summary:

Resolved issues

Commit:

7942f7203bfd549abc3e5d51badaf2d073b9e270

f21537df4942ea98ab4cafa3e2cc912b635f2113

Diff:

Revision 5 (+16 -2)

Show changes

	reviewboard/templates/notifications/review_request_email.html
	reviewboard/templates/notifications/review_request_email.txt

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-3.0.x (6f4a415)

~		With a lot of review request emails, it would be easier to prioritize requests if the email contained the branch
	~	Inclusion of branch in review request emails to aid request prioritization
-		that the change occured on.
-
-		This change includes the branch name in review request emails.

~		Testing was done manually. I checked for the branch name in review request emails after the change was made.
	~	Newly generated views were checked for the inclusion of the branch field
-		All review request emails contained the branch name.

~		Inclusion of branch in review request emails to aid request prioritization
	~	With a lot of review request emails, prioritizing review requests emails
	+	becomes uneasy.
	+
	+	Review request e-mails will now contain a branch field.

~		Newly generated views were checked for the inclusion of the branch field
	~	Tested by manually checking generated review request emails for the
	+	inclusion of the branch field.