Summary

Add latest Diff timestamp. Review last activity does not represent diff activity.

Review Request #4898 — Created Nov. 5, 2013 and submitted Dec. 8, 2013, 4:48 p.m.

Information

Owner

Valdis

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Review last activity does not represent diff activity. One can not see if the newer Diff has been uploaded or just comment added.

Testing Done

Tested manually on my development RB install.

Files

Issues

Description	From	Last Updated
Okay, one more change I noticed. Can you wrap this in {% blocktrans %} like the blocks above it?	david	Nov. 27, 2013, 1:28 p.m.
Why is this using a custom timestamp format when the others aren't? It should be consistent.	chipx86	Nov. 28, 2013, 12:37 a.m.
"Diff" shouldn't be capitalized. I'm also not sure we should be outputting both the absolute and relative timestamps here, since …	chipx86	Nov. 28, 2013, 12:40 a.m.

Branch:

+	master

Ship it!

```
Ship It!
```

Vlad,

I'm sorry for the delay. This patch doesn't apply cleanly to master anymore. Would you mind rebasing it and updating this review request?

Valdis Nov. 27, 2013, 12:28 p.m.

Hi David,
I have uploaded newer diff (r3). I hope I made rebase right.

Diff:

Revision 2 (+2 -7)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/reviews/review_request_box.html

Change Summary:

New diff (post merge)

Diff:

Revision 3 (+7 -2)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/reviews/review_request_box.html

reviewboard/templates/reviews/review_request_box.html (Diff revision 3)

Show all issues

Okay, one more change I noticed. Can you wrap this in {% blocktrans %} like the blocks above it?

VA

Valdis Nov. 27, 2013, 1:28 p.m.
```
ok, done. New diff uploaded.
```

Diff:

Revision 4 (+9 -2)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/reviews/review_request_box.html

Can you provide a screenshot of how this looks? Preferably on a 2.0 beta build, as we're unlikely to change any layout of the review request page for 1.7.x.

VA

Valdis Nov. 29, 2013, 5:08 a.m.
```
OK, the screenshot for 2.0 b2 uploaded.
```

reviewboard/templates/reviews/review_request_box.html (Diff revision 4)

Show all issues

Why is this using a custom timestamp format when the others aren't? It should be consistent.

Valdis Nov. 28, 2013, 12:42 a.m.

H:MM p.m. time format is not really world wide. Though for the consistency I changed "N j, Y, H:i T" to "F jS, Y, P T" which is more common in the RB code.

chipx86 Nov. 28, 2013, 10:27 p.m.

It shouldn't specify a format string at all. If you need customization, there's a Django settings.* variable for it.

Valdis Nov. 29, 2013, 5:08 a.m.

OK, the format string and "date" filter completely removed. The diff updated. I found other "F jS, Y, P T" strings by grepping RB source code.

reviewboard/templates/reviews/review_request_box.html (Diff revision 4)

Show all issues

"Diff" shouldn't be capitalized.

I'm also not sure we should be outputting both the absolute and relative timestamps here, since we don't elsewhere.

Valdis Nov. 28, 2013, 12:42 a.m.

ok, "Diff" has beeb lowercased. Yes, I had the same dillemma. Well, it's up to you to decide, I am only insisting on absolute timestamp which is precise even days after.

chipx86 Nov. 28, 2013, 10:27 p.m.

Mousing over a relative timestamp shows the absolute one.

Valdis Nov. 29, 2013, 5:08 a.m.

wow.. This feature works like an easter egg. I would not say it is obvious or handy.

Valdis Dec. 5, 2013, midnight

OK, I am ready to remove the absolute timestamp for consistency and leave only relative one.