Summary

Collapsing Old Reviews on Review Requests

Review Request #8489 — Created Oct. 25, 2016 and discarded Oct. 18, 2017, 3:55 a.m.

Information

Owner

Connor-Y

Repository

Review Board

Branch

master

Bugs

Depends On

~~8536~~

Reviewers

Groups

reviewboard, students

People

Description

We want to promote discussion and foster ease of use for the site.
On active or old review requests it can be hard to track whats happening.
This can be helped by collapsing inactive reviews.
This makes it easier to browse and see whats new.

This is done by implementing logic such that:
1. If the review hasn't been seen (by this user), show it
2. If the review is new, show it
3. If there is a new reply, show the review
3. If the user has a draft for that review, show it
4. If there are open issues for the review, show it
5. Otherwise, hide it (via collapsing)

Testing Done

Tested by hand to make sure everything works
Unit tests written:
1. Test old review
2. Test new review
3. Test review with old reply
4. Test review with new reply
5. Test review with draft for user
6. Test review with draft for another user
7. Test review with open issue
8. Test all of the above when logged out
9. Test all of the above with user who hasn't visited before
10. Repeat test with same user now that they've visited

Issues

Description	From	Last Updated
Col: 5 E265 block comment should start with '# '	reviewbot	Nov. 5, 2016, 1:04 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 80 E501 line too long (96 > 79 characters)	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 5 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 23 E261 at least two spaces before inline comment	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 24 E262 inline comment should start with '# '	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 22 E261 at least two spaces before inline comment	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 23 E262 inline comment should start with '# '	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 5 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 26 E261 at least two spaces before inline comment	reviewbot	Nov. 18, 2016, 4:34 p.m.
Col: 27 E262 inline comment should start with '# '	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 80 E501 line too long (93 > 79 characters)	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 21 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 17 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 80 E501 line too long (105 > 79 characters)	reviewbot	Nov. 18, 2016, 4:35 p.m.
undefined name 'timestamp'	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:35 p.m.
Col: 80 E501 line too long (96 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (95 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (137 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
undefined name 'datetime'	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 82 W291 trailing whitespace	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 22 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (97 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 5, 2016, 1:07 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 5, 2016, 1:06 p.m.
Col: 80 W291 trailing whitespace	reviewbot	Nov. 5, 2016, 1:05 p.m.
Col: 22 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:36 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 22 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:37 p.m.
'datetime' imported but unused	reviewbot	Nov. 18, 2016, 4:43 p.m.
'ReviewRequest' imported but unused	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
local variable 'comment_text_1' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:26 p.m.
local variable 'comment_text_2' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:26 p.m.
local variable 'comment_text_3' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:26 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:26 p.m.
local variable 'filediff' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:27 p.m.
local variable 'user2' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:43 p.m.
local variable 'review_one' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:27 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:27 p.m.
local variable 'review_two' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:27 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:27 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Nov. 18, 2016, 4:28 p.m.
local variable 'reply_for_three' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:28 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:28 p.m.
local variable 'reply_for_four' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:28 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:28 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:29 p.m.
timestamps.	brennie	Nov. 18, 2016, 4:30 p.m.
Instead of keeping track of i manually, you can do: for i, review in enumerate(review_request.reviews.all()): Review.objects.filter(pk=review.pk).update(timestamp=timestamp[i])	brennie	Nov. 18, 2016, 4:30 p.m.
Col: 79 W291 trailing whitespace	reviewbot	Nov. 18, 2016, 4:30 p.m.
Col: 9 E303 too many blank lines (2)	reviewbot	Nov. 18, 2016, 4:31 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Nov. 18, 2016, 4:31 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:32 p.m.
Col: 9 E303 too many blank lines (2)	reviewbot	Nov. 18, 2016, 4:32 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:32 p.m.
No print() in unit tests. YOu can add a message to self.assertEqual as the third argument.	brennie	Nov. 19, 2016, 6:28 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:33 p.m.
These should be together.	brennie	Nov. 18, 2016, 4:44 p.m.
undo this.	brennie	Nov. 18, 2016, 4:20 p.m.
Missing a period.	brennie	Nov. 18, 2016, 4:19 p.m.
No blank line here.	brennie	Nov. 18, 2016, 4:19 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Nov. 18, 2016, 4:20 p.m.
isinstance(entry, ReviewEntry)	brennie	Nov. 18, 2016, 4:18 p.m.
These are only used in one place, so just put them inline.	brennie	Nov. 18, 2016, 4:21 p.m.
Col: 80 E501 line too long (86 > 79 characters)	reviewbot	Nov. 18, 2016, 4:21 p.m.
This should be if isinstance(latest_reply, basestring))	brennie	Nov. 18, 2016, 4:17 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
This should be if last_visited != 0 and isinstance(last_visited, basestring))	brennie	Nov. 18, 2016, 4:17 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:43 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:22 p.m.
Col: 80 W291 trailing whitespace	reviewbot	Nov. 18, 2016, 4:43 p.m.
This should be formatted as: collapsed = ( review.timestmap < oldest_allowed_time and (latest_reply is None or (latest_reply < oldest_allowed_time and …	brennie	Nov. 18, 2016, 4:16 p.m.
Col: 25 E127 continuation line over-indented for visual indent	reviewbot	Nov. 18, 2016, 4:23 p.m.
Col: 25 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:23 p.m.
Col: 50 W291 trailing whitespace	reviewbot	Nov. 18, 2016, 4:23 p.m.
Col: 30 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:23 p.m.
Col: 68 W291 trailing whitespace	reviewbot	Nov. 18, 2016, 4:23 p.m.
Col: 34 W503 line break before binary operator	reviewbot	Nov. 18, 2016, 4:23 p.m.
Undo this.	brennie	Nov. 18, 2016, 4:16 p.m.
from datetime import datetime, timedelta	brennie	Nov. 19, 2016, 6:28 p.m.
'datetime' imported but unused	reviewbot	Nov. 18, 2016, 4:42 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Nov. 18, 2016, 4:38 p.m.
'ReviewRequest' imported but unused	reviewbot	Nov. 18, 2016, 4:41 p.m.
Undo this.	brennie	Nov. 19, 2016, 6:29 p.m.
Col: 19 E222 multiple spaces after operator	reviewbot	Nov. 18, 2016, 4:38 p.m.
local variable 'diffset' is assigned to but never used	reviewbot	Nov. 18, 2016, 4:38 p.m.
local variable 'user2' is assigned to but never used	reviewbot	Nov. 19, 2016, 6:39 p.m.
Col: 22 E128 continuation line under-indented for visual indent	reviewbot	Nov. 18, 2016, 4:39 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Nov. 19, 2016, 6:34 p.m.
Col: 9 E265 block comment should start with '# '	reviewbot	Nov. 18, 2016, 4:39 p.m.
These should all be in one group. Imports go as follows: from __future import unicode_literals python stdlib imports 3rdpart imports …	brennie	Nov. 19, 2016, 7:16 p.m.
isinstance(i, cls) returns a bool.	brennie	Nov. 19, 2016, 3:53 p.m.
latest_reply_timestamp	brennie	Nov. 19, 2016, 6:34 p.m.
If it is not a string, what is it? If it is string or None, then you can just do …	brennie	Nov. 19, 2016, 7:16 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:40 p.m.
latest_reply_time	brennie	Nov. 19, 2016, 6:38 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:40 p.m.
W\hy would it be 0 or a string?	brennie	Nov. 19, 2016, 6:36 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 18, 2016, 4:40 p.m.
last_visited_time	brennie	Nov. 19, 2016, 6:38 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Nov. 18, 2016, 4:40 p.m.
Col: 9 E303 too many blank lines (2)	reviewbot	Nov. 18, 2016, 4:41 p.m.
Col: 69 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:49 p.m.
Col: 70 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:49 p.m.
Col: 69 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 70 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 78 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 79 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (88 > 79 characters)	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 78 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 79 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (88 > 79 characters)	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 78 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 79 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (90 > 79 characters)	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 78 E261 at least two spaces before inline comment	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 79 E262 inline comment should start with '# '	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (98 > 79 characters)	reviewbot	Nov. 19, 2016, 6:50 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Nov. 19, 2016, 6:51 p.m.
local variable 'last_visited' is assigned to but never used	reviewbot	Nov. 19, 2016, 6:52 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Nov. 19, 2016, 6:53 p.m.
Col: 29 E128 continuation line under-indented for visual indent	reviewbot	Nov. 19, 2016, 6:53 p.m.
Col: 29 E128 continuation line under-indented for visual indent	reviewbot	Nov. 19, 2016, 6:54 p.m.
Col: 26 E127 continuation line over-indented for visual indent	reviewbot	Nov. 19, 2016, 6:55 p.m.
Rename latest_reply_timestamp variable name	MB mbalogh	Nov. 21, 2016, 2:07 p.m.
I will say use review_timestamp since you declared it.	LA larmiej	Nov. 25, 2016, 12:58 p.m.
This comment doesn't make sense to me. Can you flesh it out?	david	Nov. 27, 2016, 11:28 a.m.
Comments should include periods at the end. Several other instances in this same method.	david	Nov. 27, 2016, 10:47 a.m.
This can use zip. You're also not saving any queries by using .update() in this way. How about: for review, …	david	Dec. 1, 2016, 11:45 a.m.
This could be: self.assertEqual( [entry.collapsed for entry in entries], expected_values)	david	Nov. 27, 2016, 11:22 a.m.
It doesn't make sense to use indices here. for entry in entries: self.assertEqual(entry.collapsed, False)	david	Nov. 27, 2016, 11:21 a.m.
Same comment as above re: direct iteration.	david	Nov. 27, 2016, 11:21 a.m.
Same comment as above re: the expected_values check.	david	Nov. 27, 2016, 11:21 a.m.
This is adding a new query for something which we already are fetching below. Instead of adding this, how about …	david	Nov. 27, 2016, 9:33 a.m.
One major problem that this introduces is that a ChangeEntry or InitialStatusUpdatesEntry which contains multiple reviews (those attached to a …	david	Dec. 3, 2016, 2:43 p.m.
What is "allowed" time?	david	Nov. 27, 2016, 11:23 a.m.
In this case, it's probably cleaner to format this as: latest_reply_timestamp = \ data.latest_timestamps_by_review_id.get(review.pk)	david	Nov. 27, 2016, 11:23 a.m.
This should all not be necessary anymore, right?	david	Nov. 27, 2016, 11:23 a.m.
Draft reviews aren't shown in the review list. This can go away.	david	Nov. 27, 2016, 11:24 a.m.
Indentation is a little off here. Should be: if (last_visited_time <= review_timestamp or (latest_reply_timestamp is not None and last_visited_time <= …	david	Dec. 1, 2016, 11:59 a.m.
Too many blank lines here.	david	Nov. 27, 2016, 11:25 a.m.
Why this change?	david	Dec. 1, 2016, 10:51 a.m.
Something's weird with your change and what its parent commit is. This shouldn't be showing up in your diff.	david	Dec. 10, 2016, 5:57 p.m.
'ReviewRequestDraft' imported but unused	reviewbot	Dec. 3, 2016, 2:43 p.m.
local variable 'filediff' is assigned to but never used	reviewbot	Dec. 3, 2016, 2:43 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Dec. 3, 2016, 2:44 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 3, 2016, 2:44 p.m.
Col: 19 E222 multiple spaces after operator	reviewbot	Dec. 3, 2016, 2:44 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Dec. 3, 2016, 2:47 p.m.
This shouldn't be possible. Only top-level reviews get their own entry.	david	Dec. 8, 2016, 3:55 p.m.
This will do the wrong thing for entries that are not ReviewEntry. We should just move this assignment into the …	david	Dec. 8, 2016, 3:46 p.m.
You shouldn't need any special cases for these--they show up in reviews_entry_map.	david	Dec. 8, 2016, 3:56 p.m.
Col: 13 E303 too many blank lines (2)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 12 E114 indentation is not a multiple of four (comment)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 80 E501 line too long (91 > 79 characters)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 17 E116 unexpected indentation (comment)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 17 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 80 E501 line too long (85 > 79 characters)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 17 E116 unexpected indentation (comment)	reviewbot	Dec. 10, 2016, 12:06 p.m.
Col: 17 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E303 too many blank lines (2)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Dec. 10, 2016, 5:47 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E265 block comment should start with '# '	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 13 E303 too many blank lines (3)	reviewbot	Dec. 10, 2016, 12:07 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 1 W293 blank line contains whitespace	reviewbot	Dec. 10, 2016, 12:08 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 5:39 p.m.
Col: 80 E501 line too long (84 > 79 characters)	reviewbot	Dec. 10, 2016, 5:40 p.m.
Col: 50 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:42 p.m.
Col: 80 E501 line too long (83 > 79 characters)	reviewbot	Dec. 10, 2016, 5:42 p.m.
Col: 80 E501 line too long (82 > 79 characters)	reviewbot	Dec. 10, 2016, 5:43 p.m.
Col: 50 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:43 p.m.
Col: 40 E128 continuation line under-indented for visual indent	reviewbot	Dec. 10, 2016, 5:44 p.m.
Col: 50 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:44 p.m.
Col: 40 E128 continuation line under-indented for visual indent	reviewbot	Dec. 10, 2016, 5:44 p.m.
Col: 41 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:45 p.m.
Col: 51 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:45 p.m.
Col: 41 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:45 p.m.
Col: 50 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:45 p.m.
'StatusUpdate' imported but unused	reviewbot	Dec. 10, 2016, 5:45 p.m.
Col: 21 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:46 p.m.
Col: 21 E127 continuation line over-indented for visual indent	reviewbot	Dec. 10, 2016, 5:46 p.m.
You might want to split this up into multiple tests since it does a lot.	brennie	Dec. 11, 2016, 1 p.m.
No period for test docstrings (the test runner adds periods).	brennie	Dec. 11, 2016, 10:49 a.m.
No blank line here.	brennie	Dec. 11, 2016, 10:49 a.m.
Comments should almost always be complete sentences and they should begin with a capital.	brennie	Dec. 11, 2016, 10:50 a.m.
Why are there more than six entries here?	brennie	Dec. 11, 2016, 10:51 a.m.
Blank line between these.	brennie	Dec. 11, 2016, 10:51 a.m.
You should be building the URL using reverse(). Same elsewhere.	brennie	Dec. 12, 2016, 1:47 p.m.
Missing docstring. This test is gigantic and should be split up into multiple tests.	brennie	Dec. 12, 2016, 1:32 a.m.
datetime is a python stdlib import.	brennie	Dec. 12, 2016, 1:11 p.m.
Move the comment into the docstring.	brennie	Dec. 11, 2016, 12:35 p.m.
Blank line between these.	brennie	Dec. 11, 2016, 10:52 a.m.
Blank line between these.	brennie	Dec. 11, 2016, 10:52 a.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Dec. 11, 2016, 2:22 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Dec. 11, 2016, 2:22 p.m.
Col: 80 E501 line too long (89 > 79 characters)	reviewbot	Dec. 11, 2016, 2:22 p.m.
Col: 80 E501 line too long (92 > 79 characters)	reviewbot	Dec. 11, 2016, 2:23 p.m.
Col: 89 E226 missing whitespace around arithmetic operator	reviewbot	Dec. 11, 2016, 2:23 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 12, 2016, 1:14 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 23, 2016, 8:12 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 23, 2016, 8:12 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Dec. 23, 2016, 8:12 p.m.
Col: 5 E303 too many blank lines (3)	reviewbot	Dec. 23, 2016, 8:14 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 5
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (96 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 5
 E265 block comment should start with '# '
```

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 23
 E261 at least two spaces before inline comment

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 24
 E262 inline comment should start with '# '

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 22
 E261 at least two spaces before inline comment

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 23
 E262 inline comment should start with '# '

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 5
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 26
 E261 at least two spaces before inline comment

reviewboard/reviews/views.py (Diff revision 1)

Show all issues

Col: 27
 E262 inline comment should start with '# '

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (93 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 21
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 17
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (105 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
 undefined name 'timestamp'
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```

Change Summary:

Collapsing all reviews except:
Ones with open issues
Drafts
Recently posted - Not working for replies yet

Commit:

-	fe5ec91bbdbddc35d0852df14b02d873b495449d
+	96ba9c286c77c89aa9ff1c78136c4a58b460e42d

Diff:

Revision 2 (+90 -11)

Show changes

reviewboard/reviews/views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py

reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (96 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (95 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (137 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
 undefined name 'datetime'
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 82
 W291 trailing whitespace
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 22
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 80
 E501 line too long (97 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```

Change Summary:

Collapsing old review functionality is mostly implemented.

Commit:

-	96ba9c286c77c89aa9ff1c78136c4a58b460e42d
+	2a22adbd330459f2dc15b2ef7699e92ccb8e853c

Diff:

Revision 3 (+52 -20)

Show changes

reviewboard/reviews/views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py

reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 W291 trailing whitespace
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 22
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 3)
Show all issues
```
Col: 22
 W503 line break before binary operator
```

Change Summary:

Basic tests implemented

Branch:

-	release-3.0.x
+	master

Commit:

-	2a22adbd330459f2dc15b2ef7699e92ccb8e853c
+	c21ef08d2d70d79014fc1fdd7834fa89d5093c7e

Diff:

Revision 4 (+133 -17)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py
	reviewboard/testing/testcase.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/testing/testcase.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/testing/testcase.py
    reviewboard/reviews/tests/test_views.py


WARNING: Number of comments exceeded maximum, showing 30 of 44.

reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 'datetime' imported but unused
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 'ReviewRequest' imported but unused
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

 local variable 'comment_text_1' is assigned to but never used

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

 local variable 'comment_text_2' is assigned to but never used

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

 local variable 'comment_text_3' is assigned to but never used

reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 local variable 'filediff' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 local variable 'user2' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 local variable 'review_one' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
 local variable 'review_two' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

 local variable 'reply_for_three' is assigned to but never used

reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

 local variable 'reply_for_four' is assigned to but never used

reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 79
 W291 trailing whitespace
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 9
 E303 too many blank lines (2)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 9
 E303 too many blank lines (2)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (86 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 80
 W291 trailing whitespace
```

reviewboard/reviews/views.py (Diff revision 4)

Show all issues

Col: 25
 E127 continuation line over-indented for visual indent

reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 25
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 50
 W291 trailing whitespace
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 30
 W503 line break before binary operator
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 68
 W291 trailing whitespace
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Col: 34
 W503 line break before binary operator
```

reviewboard/reviews/tests/test_views.py (Diff revision 4)
Show all issues
```
timestamps.
```

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

Instead of keeping track of i manually, you can do:


for i, review in enumerate(review_request.reviews.all()):
    Review.objects.filter(pk=review.pk).update(timestamp=timestamp[i])

reviewboard/reviews/tests/test_views.py (Diff revision 4)

Show all issues

No print() in unit tests. YOu can add a message to self.assertEqual as the third argument.

reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
These should be together.
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
undo this.
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
Missing a period.
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
No blank line here.
```
reviewboard/reviews/views.py (Diff revision 4)
Show all issues
```
isinstance(entry, ReviewEntry)
```

reviewboard/reviews/views.py (Diff revision 4)

Show all issues

These are only used in one place, so just put them inline.

reviewboard/reviews/views.py (Diff revision 4)

Show all issues

This should be if isinstance(latest_reply, basestring))

reviewboard/reviews/views.py (Diff revision 4)

Show all issues

This should be if last_visited != 0 and isinstance(last_visited, basestring))

reviewboard/reviews/views.py (Diff revision 4)

Show all issues

This should be formatted as:

collapsed = (
    review.timestmap < oldest_allowed_time and
    (latest_reply is None or
     (latest_reply < oldest_allowed_time and
      last_visited != 0 and latest_reply < last_visited))
)

reviewboard/testing/testcase.py (Diff revision 4)
Show all issues
```
Undo this.
```

Change Summary:

Style Fixes

Commit:

-	c21ef08d2d70d79014fc1fdd7834fa89d5093c7e
+	838d49a2c898e6919e476baaae7baa32dfbce1f0

Diff:

Revision 5 (+122 -15)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
 'datetime' imported but unused
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
 'ReviewRequest' imported but unused
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
Col: 19
 E222 multiple spaces after operator
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
 local variable 'diffset' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
 local variable 'user2' is assigned to but never used
```

reviewboard/reviews/tests/test_views.py (Diff revision 5)

Show all issues

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

reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
Col: 9
 E265 block comment should start with '# '
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
Col: 9
 E303 too many blank lines (2)
```

reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
from datetime import datetime, timedelta
```
reviewboard/reviews/tests/test_views.py (Diff revision 5)
Show all issues
```
Undo this.
```

reviewboard/reviews/views.py (Diff revision 5)

Show all issues

These should all be in one group.
Imports go as follows:

from __future import unicode_literals

python stdlib imports

3rdpart imports

project imports


Imports of the form from foo import bar come after import foo. Both should be in alphabetical order.

reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
isinstance(i, cls) returns a bool.
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
latest_reply_timestamp
```

reviewboard/reviews/views.py (Diff revision 5)

Show all issues

If it is not a string, what is it?

If it is string or None, then you can just do if latest_reply:

reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
latest_reply_time
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
W\hy would it be 0 or a string?
```
reviewboard/reviews/views.py (Diff revision 5)
Show all issues
```
last_visited_time
```

Change Summary:

Tests written and working.

Commit:

-	838d49a2c898e6919e476baaae7baa32dfbce1f0
+	2b8c039719a1aa6efb8cc9b70c34382658d20b7a

Diff:

Revision 6 (+148 -14)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 69
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 70
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 69
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 70
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 78
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 79
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (88 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 78
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 79
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (88 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 78
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 79
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (90 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 78
 E261 at least two spaces before inline comment
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 79
 E262 inline comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (98 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```

reviewboard/reviews/views.py (Diff revision 6)

Show all issues

 local variable 'last_visited' is assigned to but never used

reviewboard/reviews/views.py (Diff revision 6)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```

reviewboard/reviews/views.py (Diff revision 6)

Show all issues

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

reviewboard/reviews/views.py (Diff revision 6)

Show all issues

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

reviewboard/reviews/views.py (Diff revision 6)

Show all issues

Col: 26
 E127 continuation line over-indented for visual indent

Change Summary:

Updated summary, description and testing fields.

Summary:

-	Collapsing Old Reviews on Review Requests [WIP]
+	Collapsing Old Reviews on Review Requests

Description:

~		On very active review requests, it can be hard to track what discussions (reviews, replies) are new.
~		The goal is to collapse old reviews so that it is easier to see what is new.
	~	We want to promote discussion and foster ease of use for the site.
	~	On active or old review requests it can be hard to track whats happening.
	+	This can be helped by collapsing inactive reviews.
	+	This makes it easier to browse and see whats new.
	+
	+	This is done by implementing logic such that:
	+	1. If the review hasn't been seen (by this user), show it
	+	2. If the review is new, show it
	+	3. If there is a new reply, show the review
	+	3. If the user has a draft for that review, show it
	+	4. If there are open issues for the review, show it
	+	5. Otherwise, hide it (via collapsing)

Testing Done:

	+	Tested by hand to make sure everything works
	+	Unit tests written:
	+	1. Test old review
	+	2. Test new review
	+	3. Test review with old reply
	+	4. Test review with new reply
	+	5. Test review with draft for user
	+	6. Test review with draft for another user
	+	7. Test review with open issue
	+	8. Test all of the above when logged out
	+	9. Test all of the above with user who hasn't visited before
	+	10. Repeat test with same user now that they've visited

Change Summary:

Style fixes.

Commit:

-	2b8c039719a1aa6efb8cc9b70c34382658d20b7a
+	685746adf54d6f2b2e6896139a0371731bdd3b9d

Diff:

Revision 7 (+152 -17)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

Change Summary:

Removed uneeded isInstance call.

Commit:

-	685746adf54d6f2b2e6896139a0371731bdd3b9d
+	12da016b8c7d609e0850f03c3d0f409c546adaf4

Diff:

Revision 8 (+152 -17)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/views.py (Diff revisions 7 - 8)
```
Basestring can have a default value
```
reviewboard/reviews/views.py (Diff revisions 7 - 8)
Show all issues
```
Rename latest_reply_timestamp variable name
```

Lets try this

MB

mbalogh Nov. 21, 2016, 6:26 a.m.
```
Okay, you tried it
```

chipx86 Nov. 21, 2016, 2:06 p.m.

Please use demo.reviewboard.org for testing. This server is used for production work, and your tests are interfering with that. Thanks!

```
And add a comment
```
```
Footers are great
```

reviewboard/reviews/views.py (Diff revision 8)

Why deleted these lines?

reviewboard/reviews/tests/test_views.py (Diff revision 8)

Might be missing something here, but doesn't this need to have a name like test_review_detail_collapse?

CO

Connor-Y Nov. 22, 2016, 5:34 p.m.
```
Thats probably a good idea, thanks.
```

reviewboard/reviews/views.py (Diff revision 8)

Show all issues

I will say use review_timestamp since you declared it.

Change Summary:

Made to work with fix for lastest timestamp.

Depends On:

+	~~8536 - Fix types in computation of latest timestamp of review entries.~~

Commit:

-	12da016b8c7d609e0850f03c3d0f409c546adaf4
+	68132cfe3420e10a28c1003f5be6b4b7a7b50686

Diff:

Revision 9 (+158 -21)

Show changes

	reviewboard/reviews/detail.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Change Summary:

Rebased onto master

Commit:

-	68132cfe3420e10a28c1003f5be6b4b7a7b50686
+	c80188d130fcc2ed776cac77de5a55813f7947e4

Diff:

Revision 10 (+152 -16)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 10)
Show all issues
```
This comment doesn't make sense to me. Can you flesh it out?
```

reviewboard/reviews/tests/test_views.py (Diff revision 10)

Show all issues

Comments should include periods at the end. Several other instances in this same method.

reviewboard/reviews/tests/test_views.py (Diff revision 10)

Show all issues

This can use zip. You're also not saving any queries by using .update() in this way. How about:

for review, timestamp in zip(review_request.reviews.all(),
                             timestamps):
    review.timestamp = timestamp
    review.save(update_fields=['timestamp'])

Connor-Y Nov. 27, 2016, 11:26 a.m.

This doesn't seem to properly order the reviews. The order in review_request.reviews.all() isn't necessarily sequential. I need the filter call filer(pk=review.pk) to make sure the right review gets the right timestamp.
I did switch the call to zip, but I don't think thats any faster than using enumerate.

david Nov. 27, 2016, 12:18 p.m.

You're not doing anything different with the order. enumerate will iterate through everything in the queryset in whatever order they come back in, just like zip.

Calling Review.objects.filter(pk=review.pk) is just creating a queryset that finds the object you already have.

If you want things in a guaranteed order (perhaps PK?), use review_request.reviews.order_by('pk').

Connor-Y Nov. 27, 2016, 1:12 p.m.

Oops I made a mistake. The issue isn't the ordering, its actually the call to the save() function. save() automatically sets the timestamp to timezone.now() which is overwriting my change to the timestamp. Thats why I was updating the database directly.

reviewboard/reviews/tests/test_views.py (Diff revision 10)

Show all issues

This could be:

self.assertEqual(
    [entry.collapsed for entry in entries],
    expected_values)

reviewboard/reviews/tests/test_views.py (Diff revision 10)

Show all issues

It doesn't make sense to use indices here.

for entry in entries:
    self.assertEqual(entry.collapsed, False)

reviewboard/reviews/tests/test_views.py (Diff revision 10)
Show all issues
```
Same comment as above re: direct iteration.
```
reviewboard/reviews/tests/test_views.py (Diff revision 10)
Show all issues
```
Same comment as above re: the expected_values check.
```

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

This is adding a new query for something which we already are fetching below.

Instead of adding this, how about changing it so the default value of last_visited is datetime.min, and then conditionalize the new assignment of last_visited on line 358 to only happen if visited_is_new is False?

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

One major problem that this introduces is that a ChangeEntry or InitialStatusUpdatesEntry which contains multiple reviews (those attached to a StatusUpdate) will no longer get expanded in cases where there are new replies or draft replies. Instead of looping over entries and only making changes to ReviewEntry subclasses, perhaps we can loop over data.reviews, bailing early if review.base_reply_to_id is not None.

We'd then potentially have the situation there where one review in an entry would want it expanded but another would want it collapsed. Perhaps the right solution is to start with review entries collapsed, and then conditionally expand them if there's something interesting.

reviewboard/reviews/views.py (Diff revision 10)
Show all issues
```
What is "allowed" time?
```

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

In this case, it's probably cleaner to format this as:

latest_reply_timestamp = \
    data.latest_timestamps_by_review_id.get(review.pk)

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

This should all not be necessary anymore, right?

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

Draft reviews aren't shown in the review list. This can go away.

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

Indentation is a little off here. Should be:

if (last_visited_time <= review_timestamp or
    (latest_reply_timestamp is not None and
     last_visited_time <= latest_reply_timestamp)):
    collapsed = False


It also seems like this could be combined with the "oldest allowed time" check above in some way--the code is very similar.

reviewboard/reviews/views.py (Diff revision 10)
Show all issues
```
Too many blank lines here.
```

reviewboard/reviews/views.py (Diff revision 10)

Show all issues

Why this change?

Connor-Y Nov. 27, 2016, 11:26 a.m.

This reverses the order in which reviews appear. Currently, the oldest review appears at the top of the page while the newer ones appear at the bottom. This change makes it so that the newest review appears at the top and the oldest appears at the bottom. The point is to cut down on the amount of scrolling a user has to do to reach the new (and more likely important) content.

david Nov. 27, 2016, 12:18 p.m.

Whether we should do that at all is highly debatable (a big change like that is bound to piss off a lot of users, whether or not it's actually an improvement). Even if we decided to do it, we shouldn't include it in this change (which is specifically addressing the open/close state of entries).

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py

Change Summary:

Added code to support StatusUpdates

Commit:

-	c80188d130fcc2ed776cac77de5a55813f7947e4
+	28c0495e49e5232c84f36c933ebd2a758d42a6d1

Diff:

Revision 11 (+294 -23)

Show changes

	reviewboard/reviews/detail.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py

reviewboard/reviews/tests/test_views.py (Diff revision 11)
Show all issues
```
 'ReviewRequestDraft' imported but unused
```
reviewboard/reviews/tests/test_views.py (Diff revision 11)
Show all issues
```
 local variable 'filediff' is assigned to but never used
```
reviewboard/reviews/tests/test_views.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/views.py (Diff revision 11)
Show all issues
```
Col: 19
 E222 multiple spaces after operator
```
reviewboard/reviews/views.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```

Looking at your logic, I think it's a bit overcomplicated and fragile, the way that you're special casing each type of entry.

Instead, how about:

Default every entry which contains one or more reviews to collapsed.
Iterate over every review in reviews_entry_map. If the review has a reason to not be collapsed, expand the relevant entry.

reviewboard/reviews/detail.py (Diff revision 11)

Show all issues

Something's weird with your change and what its parent commit is. This shouldn't be showing up in your diff.

reviewboard/reviews/views.py (Diff revision 11)

Show all issues

This shouldn't be possible. Only top-level reviews get their own entry.

reviewboard/reviews/views.py (Diff revision 11)

Show all issues

This will do the wrong thing for entries that are not ReviewEntry. We should just move this assignment into the if statement.

reviewboard/reviews/views.py (Diff revision 11)

Show all issues

You shouldn't need any special cases for these--they show up in reviews_entry_map.

Change Summary:

Cleaned up code logic and wrote basic tests for StatusUpdates. Still WIP

Commit:

-	28c0495e49e5232c84f36c933ebd2a758d42a6d1
+	5d455898a5d1a25f10d7ef25506603ca45f61de6

Diff:

Revision 12 (+411 -30)

Show changes

	reviewboard/reviews/detail.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py


WARNING: Number of comments exceeded maximum, showing 30 of 35.

reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E303 too many blank lines (2)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 12
 E114 indentation is not a multiple of four (comment)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (91 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 17
 E116 unexpected indentation (comment)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 17
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (85 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 17
 E116 unexpected indentation (comment)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 17
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E303 too many blank lines (2)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E265 block comment should start with '# '
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 13
 E303 too many blank lines (3)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 12)
Show all issues
```
Col: 1
 W293 blank line contains whitespace
```

Change Summary:

Removed special cases and finished tests.

Commit:

-	5d455898a5d1a25f10d7ef25506603ca45f61de6
+	7318ba96813fe30d4ac66bae840a8a78a4c850f4

Diff:

Revision 13 (+528 -24)

Show changes

	reviewboard/reviews/detail.py
	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py
    reviewboard/reviews/detail.py

reviewboard/reviews/tests/test_views.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (84 > 79 characters)
```

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 50
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (83 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (82 > 79 characters)
```

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 50
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

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

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 50
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

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

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 41
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 51
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 41
 E127 continuation line over-indented for visual indent

reviewboard/reviews/tests/test_views.py (Diff revision 13)

Show all issues

Col: 50
 E127 continuation line over-indented for visual indent

reviewboard/reviews/views.py (Diff revision 13)
Show all issues
```
 'StatusUpdate' imported but unused
```

reviewboard/reviews/views.py (Diff revision 13)

Show all issues

Col: 21
 E127 continuation line over-indented for visual indent

reviewboard/reviews/views.py (Diff revision 13)

Show all issues

Col: 21
 E127 continuation line over-indented for visual indent

Change Summary:

Rebased to master.

Commit:

-	7318ba96813fe30d4ac66bae840a8a78a4c850f4
+	25a0dc6fa3aadddb9d1af850f11faed59ba513cc

Diff:

Revision 14 (+517 -19)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 14)

Show all issues

You might want to split this up into multiple tests since it does a lot.

reviewboard/reviews/tests/test_views.py (Diff revision 14)
Show all issues
```
No period for test docstrings (the test runner adds periods).
```
reviewboard/reviews/tests/test_views.py (Diff revision 14)
Show all issues
```
No blank line here.
```

reviewboard/reviews/tests/test_views.py (Diff revision 14)

Show all issues

Comments should almost always be complete sentences and they should begin with a capital.

reviewboard/reviews/tests/test_views.py (Diff revision 14)

Show all issues

Why are there more than six entries here?

Connor-Y Dec. 11, 2016, 10:52 a.m.

We also need to set the timestamp of the replies for each review.

reviewboard/reviews/tests/test_views.py (Diff revision 14)
Show all issues
```
Blank line between these.
```

reviewboard/reviews/tests/test_views.py (Diff revision 14)

Show all issues

You should be building the URL using reverse(). Same elsewhere.

Connor-Y Dec. 11, 2016, 2:21 p.m.

The rest of the test suite seems to use this command to get the entire review request.
Also, I'm not quite sure how to use reverse() or local_site_reverse() to get the entire review request.

brennie Dec. 12, 2016, 1:31 a.m.


rsp = self.client.get(
    local_site_reverse(
        'review-request-detail',
        kwargs={
            'review_request_id': review_request.pk,
        }
    ))

reviewboard/reviews/tests/test_views.py (Diff revision 14)

Show all issues

Missing docstring.

This test is gigantic and should be split up into multiple tests.

reviewboard/reviews/views.py (Diff revision 14)

Show all issues

datetime is a python stdlib import.

Connor-Y Dec. 11, 2016, 2:21 p.m.

I actually want the datetime from the datetime library. They both have the same name and do similar things so its kinda confusing.

brennie Dec. 12, 2016, 1:31 a.m.

Yes, which is standard library. This should look like:

import logging
import time
from datetime import datetime, timedelta

# 3rd party imports

# reviewboard.* imports

reviewboard/reviews/views.py (Diff revision 14)
Show all issues
```
Move the comment into the docstring.
```
reviewboard/reviews/views.py (Diff revision 14)
Show all issues
```
Blank line between these.
```
reviewboard/reviews/views.py (Diff revision 14)
Show all issues
```
Blank line between these.
```

Change Summary:

Split the large tests into many smaller ones and added docstrings.

Commit:

-	25a0dc6fa3aadddb9d1af850f11faed59ba513cc
+	2c69aa0fca445ce66a027a530f58a22c722585d0

Diff:

Revision 15 (+525 -20)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 15)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 15)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 15)
Show all issues
```
Col: 80
 E501 line too long (89 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 15)
Show all issues
```
Col: 80
 E501 line too long (92 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 15)
Show all issues
```
Col: 89
 E226 missing whitespace around arithmetic operator
```

Change Summary:

Style Fixes.

Commit:

-	2c69aa0fca445ce66a027a530f58a22c722585d0
+	785a208caee0f97da9a1bf0ea2926c5d3190f5e0

Diff:

Revision 16 (+530 -20)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 16)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

Change Summary:

Minor fixes.

Commit:

-	785a208caee0f97da9a1bf0ea2926c5d3190f5e0
+	aeccaba6c42ac4fb5b237af0ab8ca7aad00bb818

Diff:

Revision 17 (+542 -22)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

reviewboard/reviews/tests/test_views.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
reviewboard/reviews/tests/test_views.py (Diff revision 17)
Show all issues
```
Col: 5
 E303 too many blank lines (3)
```

Change Summary:

Style Fixes

Commit:

-	aeccaba6c42ac4fb5b237af0ab8ca7aad00bb818
+	99fff3222713a2a3ad4c8496d3a28951ace72f27

Diff:

Revision 18 (+540 -22)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/tests/test_views.py

Tool: Pyflakes
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/reviews/views.py
    reviewboard/reviews/tests/test_views.py

Status: Discarded

Change Summary:

Rewrote for the new entry work as /r/9281/