Summary:

User Infobox Improvements

Review Request #7662 — Created Sept. 26, 2015 and discarded March 28, 2016, 11:25 a.m.

Information
Owner:	chronicleyu
Repository:	Review Board
Branch:	master
Bugs:
Depends On:
Reviewers
Groups:	reviewboard, students
People:	brennie, chipx86, david, mike_conley

Description

When hovering over a user's name, an info box appears showing their avatar/photo, name, e-mail address, and some other information. It's somewhat useful, but could be more useful.
This change adds a couple new pieces of information:

1) Their timezone and how many hours ahead/behind you they are (to help determine if they'd be available at the moment).
2) Some review request stats (how many review requests they have been assigned to, and how many they have had out for review).

A new UserInfoExtensionHook is also created to provide additional information, and associated entity tag data, for this box, should the need arise. That way, third-parties can integrate other services they use with this box. Documentation and example usage for the new hook are also provided.

Testing Done

Moused over usernames to verify that the infobox appears with the new (and old) pieces of information displays correctly.
Overrode get_extra_context to ensure that additional information added via the extension hook displays correctly.

Files

Issues

Description	From	Last Updated
Just saying how far ahead/behind forces me to do math. Can we imitate what Slack does? They have: <clock symbol> …	david	Oct. 17, 2015, 11:21 a.m.
There's a mismatch in styles here. We should aim for consistency, or separate them out more by whitespace. Also, the …	chipx86	Oct. 20, 2015, 2:54 p.m.
Now that we're showing this using a time notation, we can probably skip the word "hours" ("4:00 hours" seems weird, …	david	Oct. 31, 2015, 7:22 a.m.
'tzinfo' imported but unused	reviewbot	Oct. 6, 2015, 3:14 p.m.
'timedelta' imported but unused	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 47 E231 missing whitespace after ','	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 22 E261 at least two spaces before inline comment	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 64 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 31 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 34 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 37 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 40 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 60 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 27 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 30 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 33 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 36 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 24 E261 at least two spaces before inline comment	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 64 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 26 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:14 p.m.
Col: 32 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 35 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 60 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 26 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 32 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 35 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 10 E261 at least two spaces before inline comment	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 64 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 75 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 13 E122 continuation line missing indentation or outdented	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 60 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 75 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
These imports need to be broken up. We organize our imports as following: from __future__ import unicode_literals # Standard library …	brennie	Oct. 8, 2015, 11:44 a.m.
'django' imported but unused	reviewbot	Oct. 6, 2015, 3:15 p.m.
No blank line here.	brennie	Oct. 6, 2015, 3:20 p.m.
Col: 32 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:15 p.m.
Col: 28 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:16 p.m.
Col: 36 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:16 p.m.
Col: 42 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 6, 2015, 3:16 p.m.
This should be six.text_type (i.e., unicode) instead of str. Also This will have to use fomratting so we can localize …	brennie	Oct. 8, 2015, 11:44 a.m.
Col: 36 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:16 p.m.
Col: 5 E303 too many blank lines (3)	reviewbot	Oct. 6, 2015, 3:16 p.m.
Col: 57 E225 missing whitespace around operator	reviewbot	Oct. 6, 2015, 3:16 p.m.
I would break this up as: 'users_stats': { 'assigned': num_requests_assigned, 'posted': num_requests_posted, # ... } Then you can do the …	brennie	Oct. 6, 2015, 3:20 p.m.
Col: 1 W391 blank line at end of file	reviewbot	Oct. 6, 2015, 3:16 p.m.
Col: 15 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 15 E126 continuation line over-indented for hanging indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 15 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 15 E126 continuation line over-indented for hanging indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 15 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 15 E126 continuation line over-indented for hanging indent	reviewbot	Oct. 8, 2015, 12:05 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:18 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:18 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Oct. 8, 2015, 12:18 p.m.
No need to sign your work here. :)	mike_conley	Oct. 17, 2015, 11:22 a.m.
This is essentially the same value as requsr_dttemp. Can't we re-use the same value for both, and just call it …	mike_conley	Oct. 17, 2015, 11:22 a.m.
Converting the UTC datetime into a string and then extracting things from the string and converting them back into integers …	mike_conley	Oct. 17, 2015, 11:22 a.m.
I think it'd maybe make more sense do put the tz and hours values into the context and pass them …	mike_conley	Oct. 28, 2015, 11:38 a.m.
We already have the user, no?	mike_conley	Oct. 17, 2015, 11:23 a.m.
It seems to me we should be able to narrow down the review_requests with a QuerySet to find the cases …	mike_conley	Oct. 22, 2015, 4 p.m.
I think we should be fine to pass these down to the template as ints.	mike_conley	Oct. 22, 2015, 3:44 p.m.
"Assigned:" and "Posted:" aren't going to be translated. Let's send down just the raw integers, and do the translation in …	mike_conley	Oct. 22, 2015, 3:44 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 17, 2015, 11:21 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 17, 2015, 11:21 a.m.
undefined name 'e'	reviewbot	Oct. 17, 2015, 11:21 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Oct. 17, 2015, 11:21 a.m.
Col: 13 W292 no newline at end of file	reviewbot	Oct. 17, 2015, 11:21 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 58 E251 unexpected spaces around keyword / parameter equals	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 60 E251 unexpected spaces around keyword / parameter equals	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 22 E225 missing whitespace around operator	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 5 E303 too many blank lines (2)	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 5 E265 block comment should start with '# '	reviewbot	Oct. 17, 2015, 12:18 p.m.
Col: 28 E226 missing whitespace around arithmetic operator	reviewbot	Oct. 17, 2015, 12:18 p.m.
"HTML"	chipx86	Oct. 22, 2015, 3:39 p.m.
A couple things: 1) When showing examples like this, we want them in literal blocks. 2) I have no idea …	chipx86	Oct. 20, 2015, 2:54 p.m.
This shouldn't be here.	chipx86	Oct. 20, 2015, 2:54 p.m.
These docs, just like the ones above, will be generated and made available on our site. This needs to be …	chipx86	Oct. 20, 2015, 2:54 p.m.
No blank line here.	chipx86	Oct. 20, 2015, 2:54 p.m.
This shouldn't take a list of sections. That's going to lock us into a particular design, and it's going to …	chipx86	Oct. 20, 2015, 2:54 p.m.
Missing a docstring. No blank line here.	chipx86	Oct. 20, 2015, 2:55 p.m.
Two blank lines here.	chipx86	Oct. 20, 2015, 2:55 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	Oct. 20, 2015, 2:54 p.m.
This is a third-party module.	chipx86	Oct. 22, 2015, 4 p.m.
This must be inserted alphabetically.	chipx86	Oct. 20, 2015, 2:55 p.m.
This blank line must remain.	chipx86	Oct. 22, 2015, 3:44 p.m.
I'm going to want to see all this logic heavily documented, so every bit of it is clear.	chipx86	Oct. 20, 2015, 2:56 p.m.
What's all this?	chipx86	Oct. 22, 2015, 3:44 p.m.
<html> means the start of a document. We never want that, or we'll confuse browsers.	chipx86	Oct. 20, 2015, 2:58 p.m.
On a large deployment, this may produce thousands or tens of thousands of review requests. Since we're just counting, we …	chipx86	Oct. 22, 2015, 4 p.m.
Blank line between these.	chipx86	Oct. 20, 2015, 2:58 p.m.
Missing a trailing comma.	chipx86	Oct. 20, 2015, 2:58 p.m.
These should be one statemnet.	chipx86	Oct. 20, 2015, 2:59 p.m.
Make sure all the content aligns properly with the current indentation level.	chipx86	Oct. 20, 2015, 2:59 p.m.
'format_html' imported but unused	reviewbot	Oct. 20, 2015, 3:18 p.m.
'mark_safe' imported but unused	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 39 E231 missing whitespace after ','	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 9 E303 too many blank lines (2)	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 13 E123 closing bracket does not match indentation of opening bracket's line	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 43 E231 missing whitespace after ','	reviewbot	Oct. 20, 2015, 3:18 p.m.
Col: 51 E202 whitespace before ')'	reviewbot	Oct. 22, 2015, 4:09 p.m.
This should start with .. code-block:: python	david	Oct. 26, 2015, 11:36 a.m.
Docstrings should start with a 1-line summary, then a blank line, then additional paragraphs of explanation.	david	Oct. 27, 2015, 11:17 a.m.
We should check that template_name is a string, because otherwise we'll call render_to_string and pass it None. Alternatively, just get …	david	Oct. 26, 2015, 11:57 a.m.
ExtensionHook doesn't define get_extra_context so the super call is superfluous and useless.	david	Oct. 26, 2015, 11:36 a.m.
Remove this line.	david	Oct. 26, 2015, 11:36 a.m.
We should just include the <span> in the template, instead of passing it in like this.	david	Oct. 28, 2015, 11:30 a.m.
Please put this line back.	david	Oct. 26, 2015, 11:36 a.m.
'format_html' imported but unused	reviewbot	Oct. 28, 2015, 11:56 a.m.
This should line up with the c in code-block.	brennie	Oct. 28, 2015, 12:32 p.m.
This should line up with the c in code-block.	brennie	Oct. 28, 2015, 12:32 p.m.
Can you reformat this as follows? raise ValueError( 'Invalid UserInfoBox section: %s; must be one of %s' % (section, ', …	brennie	Oct. 28, 2015, 12:33 p.m.
Can you reformat this as: return render_to_string(self.template_name, RequestContext(request, context)) It's easier to read this way.	brennie	Oct. 28, 2015, 12:35 p.m.
Remove this line.	brennie	Oct. 28, 2015, 12:35 p.m.
This comment is unnecessary.	brennie	Oct. 28, 2015, 12:35 p.m.
Single quotes. Also, this should be six.text_type, not str: from django.utils import six The reason we use this is that …	brennie	Oct. 28, 2015, 12:37 p.m.
Can you put UserInfoBoxHook.render on one line? that way its clear there is not supposed to be a space between …	brennie	Oct. 28, 2015, 12:39 p.m.
This should be '... extension: "', extension.id, e, exc_info=True) (I know some places use exc_info=1, but we aren't doing that …	brennie	Oct. 28, 2015, 12:52 p.m.
This blank line isn't necessary when there's no docstring.	david	Oct. 29, 2015, 5:26 a.m.
I'm not sure what "custom parameters..." means here, and it's not something that users can change (since render chooses what …	david	Oct. 29, 2015, 5:29 a.m.
get_extra_context should return a dict, not a str.	david	Oct. 29, 2015, 5:29 a.m.
Col: 25 E128 continuation line under-indented for visual indent	reviewbot	Oct. 29, 2015, 5:26 a.m.
There are fractional time zones that aren't 30 minutes (for example, Kathmandu, Nepal is GMT +5:45). I don't think this …	david	Oct. 29, 2015, 5:53 a.m.
We should pass the raw numbers in to the template and put the text there.	david	Oct. 29, 2015, 6:19 a.m.
As is, this won't include any of the arguments because the first argument doesn't have any format specifiers. You can …	david	Oct. 29, 2015, 5:35 a.m.
Col: 30 E201 whitespace after '('	reviewbot	Oct. 29, 2015, 6:39 a.m.
local variable 'e' is assigned to but never used	reviewbot	Oct. 29, 2015, 6:39 a.m.
How about the following: # We round the timezones while minutes >= 10: if ahead: hr_diff += 0.25 else: hr_diff …	brennie	Oct. 29, 2015, 4:16 p.m.
elif	brennie	Oct. 29, 2015, 11:53 a.m.
When interpolating only one variable, you don't need to use dict interpolation.	brennie	Oct. 29, 2015, 12:10 p.m.
So string additions are slow. How about: custom_info_parts = [] for hook in UserInfoBoxHooks.hooks: try: custom_info_parts.append(hook.render(user, request)) except Exception: # …	brennie	Oct. 29, 2015, 12:16 p.m.
We want to make sure that this isn't going to be escaped incorrectly. We should be checking if the result …	brennie	Oct. 29, 2015, 12:25 p.m.
extension.id on next line.	brennie	Oct. 29, 2015, 11:57 a.m.
Can we move all of this computation to happen after the etag_if_none_match call? We don't want to do this if …	david	Nov. 2, 2015, 11:35 a.m.
I suspect we should show something else (or not show this at all) if request.user is the same as user. …	david	Nov. 2, 2015, 10:37 a.m.
Can we localize this? We also should be able to skip the six.text_type call. td_msg = _('Same local time as …	david	Oct. 31, 2015, 7:24 a.m.
I know the existing text in here isn't localized, but we should mark these words (and the others in here) …	david	Nov. 2, 2015, 11:47 a.m.
If you do: :py:class:`~reviewboard.extensions.hooks.UserInfoHook` it will still link to the right class, but it will render as UserInfoHook.	brennie	Nov. 14, 2015, 7:53 a.m.
get_extra_context should return a rst :py:class:`dict`.	brennie	Nov. 14, 2015, 7:54 a.m.
Double backquotes	brennie	Nov. 14, 2015, 7:55 a.m.
Double backquotes	brennie	Nov. 14, 2015, 7:55 a.m.
No blank line here.	brennie	Nov. 14, 2015, 7:55 a.m.
six.text_type, not unicode. (from django.util import six)	brennie	Nov. 14, 2015, 7:56 a.m.
How about: review_requests_sent = ( ReviewRequest.objects .public(user=request.user) .filter(submitter=user) .count() ) Does this include ALL sent rrs or only the open …	brennie	Nov. 14, 2015, 8:18 a.m.
Why are you casting this to a string? Does this include ALL assigned rrs or only open?	brennie	Nov. 14, 2015, 8:18 a.m.
Maybe we should we leave it up to the hook to return their strings as marked safe?	brennie	Nov. 21, 2015, 8:52 a.m.
Col: 13 E131 continuation line unaligned for hanging indent	reviewbot	Nov. 21, 2015, 9:01 a.m.
"A :py:class:`~reviewboard.extensions.hooks.UserInfoBoxHook` can be used..."	brennie	Nov. 24, 2015, 9:14 a.m.
Blank space after comma	brennie	Nov. 24, 2015, 9:15 a.m.
"to return a :py:class:`dict`. The result of this function will be used, in addition to the request and user, for …	brennie	Nov. 24, 2015, 9:24 a.m.
Should comment out the dots because its not valid python.	brennie	Nov. 24, 2015, 9:25 a.m.
Provide an actual example.	brennie	Nov. 24, 2015, 1:13 p.m.
backquotes around the strings, e.g. * ``'user-identity'`` * ``'extra-details'``	brennie	Nov. 24, 2015, 9:30 a.m.
If required, :py:meth:`get_extra_context` may be overridden ...	brennie	Nov. 24, 2015, 9:31 a.m.
If required, :py:meth:`get_extra_context` may be overridden ...	brennie	Nov. 24, 2015, 9:31 a.m.
Can we call this SECTIONS instead?	brennie	Nov. 24, 2015, 9:35 a.m.
Col: 33 E128 continuation line under-indented for visual indent	reviewbot	Nov. 24, 2015, 9:12 a.m.
All comments should be complete sentences and should end in periods. This goes for all comments in this method.	brennie	Nov. 24, 2015, 10 a.m.
requser? How about target_user?	brennie	Nov. 24, 2015, 10:02 a.m.
Instead of my_user, lets call it user or local_user	brennie	Nov. 24, 2015, 10:02 a.m.
This isn't a very useful comment.	brennie	Nov. 24, 2015, 10:02 a.m.
This is not a complete sentence.	brennie	Nov. 24, 2015, 10:03 a.m.
We should probably wrap this in an {% if custom_info %}. Also class="custom-info"	brennie	Nov. 24, 2015, 10:13 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	Nov. 24, 2015, 2:40 p.m.
We should call this SECTIONS	david	Nov. 25, 2015, 11:55 a.m.
We should indent this as: return mark_safe( render_to_string(self.template_name, RequestContext(request, context)))	david	Nov. 25, 2015, 12:02 p.m.
Col: 17 E128 continuation line under-indented for visual indent	reviewbot	Nov. 25, 2015, 11:54 a.m.
There's no need to cast this to a string.	david	Nov. 25, 2015, 12:02 p.m.
We don't seem to be splitting this into the different sections.	david	Nov. 27, 2015, 8:18 a.m.
The etag needs to take into account the time difference and stats, and probably the extension hook stuff as well. …	brennie	Dec. 2, 2015, 11:25 a.m.
According to the raw diff, you removed a newline from the last line. Please add it back.	brennie	Dec. 7, 2015, 7:49 a.m.
Unnecessary.	brennie	Dec. 7, 2015, 7:09 a.m.
Can we just call this user_tz?	brennie	Dec. 7, 2015, 7:17 a.m.
Unnecessary	brennie	Dec. 7, 2015, 7:09 a.m.
This isn't the entity tag. You have to call encode_etag on the result.	brennie	Dec. 7, 2015, 7:22 a.m.
Can you just put one, larger comment above the timezone calculation stuff? We don't need comments every few lines -- …	brennie	Dec. 7, 2015, 7:10 a.m.
Unnecessary.	brennie	Dec. 7, 2015, 7:11 a.m.
Unnecessary.	brennie	Dec. 7, 2015, 7:12 a.m.
No blank line here.	brennie	Dec. 7, 2015, 7:12 a.m.
Unnecessary.	brennie	Dec. 7, 2015, 7:12 a.m.
Get.	brennie	Dec. 7, 2015, 7:12 a.m.
Unnecessary.	brennie	Dec. 7, 2015, 7:13 a.m.
"custom rendering context"	brennie	Dec. 7, 2015, 10:32 a.m.
"In this case, the get_etag_data should be overridden to return data that will be used to generate the entity tag …	brennie	Dec. 7, 2015, 10:35 a.m.
"Get"	brennie	Dec. 7, 2015, 10:35 a.m.
Remove this.	brennie	Dec. 7, 2015, 10:35 a.m.

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html


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

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
 'tzinfo' imported but unused
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
 'timedelta' imported but unused
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 47
 E231 missing whitespace after ','
```
reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 5
 E303 too many blank lines (2)
```

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: 64
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 31
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 34
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 37
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 40
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 60
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 27
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 30
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 33
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 36
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 24
 E261 at least two spaces before inline comment

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

Show all issues

Col: 64
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 26
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 29
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 32
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 35
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 60
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 26
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 29
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 32
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 35
 E226 missing whitespace around arithmetic operator

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

Show all issues

Col: 10
 E261 at least two spaces before inline comment

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

Show all issues

Col: 64
 E226 missing whitespace around arithmetic operator

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 75
 E225 missing whitespace around operator
```

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

Show all issues

Col: 13
 E122 continuation line missing indentation or outdented

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

Show all issues

Col: 60
 E226 missing whitespace around arithmetic operator

reviewboard/reviews/views.py (Diff revision 1)
Show all issues
```
Col: 75
 E225 missing whitespace around operator
```

Change Summary:

Reworked hour difference code to use the "datetime, ptyz, and utcoffset" method, halfway through working on user's RR stats component of the project

Description:

		WIP: Created and implemented algorithm to display time difference, experimented with utcoffset as an alternative.

		Before trying out the aforementioned algorithm, I first coded it in a separate .py file (attached as main.py), and then I wrote an exhaustive test (attached as test.py). The log file from that test is too big to attach here (50MB), but it is uploaded to Google Drive (https://drive.google.com/file/d/0B0qLz3PLbZZIX0h4TlllN1RydlE/view?usp=sharing). In addition to the exhaustive test, I also ran some individual tests for a shorter log file (attached here as rb_hr_indiv_tests.txt).
	+
	+	UPDATE: The code now uses the "datetime, ptyz, and utcoffset" method rather than the algorithm I created.

Testing Done:

~		Hovering over a self-created test account's username on a local test server. The user infobox will show the result of the algorithm (until I can get utcoffset to work properly).
	~	Hovering over a self-created test account's username on a local test server. UPDATE: The user infobox will now display result of the "datetime, ptyz, and utcoffset" method, not the algorithm.

Diff:

Revision 2 (+133)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

Removed Files:

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
 'django' imported but unused
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 32
 E225 missing whitespace around operator
```

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

Show all issues

Col: 28
 E226 missing whitespace around arithmetic operator

reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 36
 E225 missing whitespace around operator
```

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

Show all issues

Col: 42
 E226 missing whitespace around arithmetic operator

reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 36
 E225 missing whitespace around operator
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 5
 E303 too many blank lines (3)
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 57
 E225 missing whitespace around operator
```
reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
Col: 1
 W391 blank line at end of file
```

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

Show all issues

These imports need to be broken up. We organize our imports as following:


from __future__ import unicode_literals

# Standard library imports like logging, time, etc.

# Third-party library imports like django, pytz, etc.

# reviewboard.* imports

chronicleyu Oct. 7, 2015, 6:41 a.m.

It only uses pytz and datetime now. Is this a 'fix' or a 'drop'?

brennie Oct. 7, 2015, 12:03 p.m.
```
It would be a fix.
```

reviewboard/reviews/views.py (Diff revision 2)
Show all issues
```
No blank line here.
```

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

Show all issues

This should be six.text_type (i.e., unicode) instead of str.
Also This will have to use fomratting so we can localize the strings, e.g.

from django.utils.translation import ugettext as _

# ...

if hr_diff < 0:
    td_msg = (_('%(tz)s: %(hours)s hours ahead of you')
              % {
                  'tz': requsr_tz,
                  'hours': -1 * hr_diff
              })
elif hr_diff > 0:
    # ...

CH

chronicleyu Oct. 7, 2015, 6:41 a.m.
```
Missed this one. Will get on this asap.
```

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

Show all issues

I would break this up as:

'users_stats': {
    'assigned': num_requests_assigned,
    'posted': num_requests_posted,
    # ...
}

Then you can do the formatting in the infobox with {% blocktrans %} .... {% endblocktrans %} so that it gets localized.

Change Summary:

Completed the functionality to display user RR statistics.

Description:

~		WIP: Created and implemented algorithm to display time difference, experimented with utcoffset as an alternative.
~
~		Before trying out the aforementioned algorithm, I first coded it in a separate .py file (attached as main.py), and then I wrote an exhaustive test (attached as test.py). The log file from that test is too big to attach here (50MB), but it is uploaded to Google Drive (https://drive.google.com/file/d/0B0qLz3PLbZZIX0h4TlllN1RydlE/view?usp=sharing). In addition to the exhaustive test, I also ran some individual tests for a shorter log file (attached here as rb_hr_indiv_tests.txt).
~
	~	Adds three pieces of information to the user info box:
	~	1) Hour difference with local computer
	~	2) User's review requests
	~	3) User's assigned requests
-		UPDATE: The code now uses the "datetime, ptyz, and utcoffset" method rather than the algorithm I created.

Testing Done:

~		Hovering over a self-created test account's username on a local test server. UPDATE: The user infobox will now display result of the "datetime, ptyz, and utcoffset" method, not the algorithm.
	~	Hovering over a test account usernames on a local test server.

Diff:

Revision 3 (+72 -3)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

Change Summary:

Changed td_msg to be six.text_type instead of an str.

Diff:

Revision 4 (+81 -3)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

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

Show all issues

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

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

Show all issues

Col: 15
 E126 continuation line over-indented for hanging indent

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

Show all issues

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

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

Show all issues

Col: 15
 E126 continuation line over-indented for hanging indent

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

Show all issues

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

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

Show all issues

Col: 15
 E126 continuation line over-indented for hanging indent

Change Summary:

Fixed the td_msg indentation issues brought up by ReviewBot

Diff:

Revision 5 (+84 -3)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

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

Show all issues

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

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

Show all issues

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

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

Show all issues

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

Change Summary:

More fixes on the td_msg indentation issues.

Diff:

Revision 6 (+84 -3)

Show changes

	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

Change Summary:

Changed the title of this review request. No longer a code dump.

Summary:

-	WIP: User Infobox Improvements Code Dump
+	User Infobox Improvements

```
Can you please attach screenshots?
```
1. CH
  
  chronicleyu Oct. 9, 2015, 11:41 a.m.
  Done.

Change Summary:

Added screenshots of the info box.

Added Files:

I haven't really looked at the code yet, but I have one comment on the UI:

Close up of the info box (Revision 1)

Show all issues

Just saying how far ahead/behind forces me to do math.
Can we imitate what Slack does? They have:
<clock symbol> 3:10PM local time / 3 hours ahead of you (EST)

Change Summary:

WIP: Adding UserInfoBoxExtensionHook

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests
	+
	+	WIP: Adds a UserInfoBoxHook to allow for more information to be added.

Diff:

Revision 7 (+121 -4)

Show changes

	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html

reviewboard/extensions/hooks.py (Diff revision 7)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 7)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 7)
Show all issues
```
 undefined name 'e'
```
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 7)
Show all issues
```
Col: 13
 E128 continuation line under-indented for visual indent
```
reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 7)
Show all issues
```
Col: 13
 W292 no newline at end of file
```

reviewboard/reviews/views.py (Diff revision 6)
Show all issues
```
No need to sign your work here. :)
```
1. CH
  
  chronicleyu Oct. 17, 2015, 11:23 a.m.
  Just making it easier to ctrl-f to my code :)

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

Show all issues

This is essentially the same value as requsr_dttemp.

Can't we re-use the same value for both, and just call it "now" or "utc_now"?

utc_now = datetime.datetime.now(pytz.utc)

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

Show all issues

Converting the UTC datetime into a string and then extracting things from the string and converting them back into integers to do math seems suboptimal.
I'm reasonably certain that we can extract the values that we need from the return value of datetime.now(pytz.utc) and similar.
Example:

now = datetime.datetime.now(pytz.utc)

other_user_tz = pytz.timezone(user.get_profile().timezone)
other_user_time = now.astimezone(other_user_tz)

my_user_tz = pytz.timezone(request.user.get_profile().timezone)
my_user_time = now.astimezone(my_user_tz)


Can we not then get the timedelta's that we want in seconds, and then convert to minutes / hours, to generate the values that we need?
Also, you might find this interesting: http://stackoverflow.com/questions/14190045/how-to-convert-datetime-timedelta-to-minutes-hours-in-python

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

Show all issues

I think it'd maybe make more sense do put the tz and hours values into the context and pass them into the template, and then insert them with the translation strings in the template, instead of just sending down the translated strings from the view.

chronicleyu Oct. 22, 2015, 4:03 p.m.

The way the timezone and hours are calculated has undergone major changes since. Is this issue still applicable?

reviewboard/reviews/views.py (Diff revision 6)
Show all issues
```
We already have the user, no?
```

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

Show all issues

It seems to me we should be able to narrow down the review_requests with a QuerySet to find the cases where rr.submitter == user. QuerySet docs: https://docs.djangoproject.com/en/1.6/topics/db/queries/

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

Show all issues

I think we should be fine to pass these down to the template as ints.

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

Show all issues

"Assigned:" and "Posted:" aren't going to be translated.

Let's send down just the raw integers, and do the translation in the views.

Change Summary:

Added extension hook and documentation for using the hook. Made changes to UI and calculation method for time difference

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

~		WIP: Adds a UserInfoBoxHook to allow for more information to be added.
	~	Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook.
	+
	+	NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.

Testing Done:

~		Hovering over a test account usernames on a local test server.
	~	Hovering over a test account usernames on a local test server.
	+	Unit testing for the extension hook.

Diff:

Revision 8 (+304 -7)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst~
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

Removed Files:

Added Files:

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/index.rst~



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/index.rst~

reviewboard/extensions/hooks.py (Diff revision 8)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

reviewboard/extensions/hooks.py (Diff revision 8)

Show all issues

Col: 58
 E251 unexpected spaces around keyword / parameter equals

reviewboard/extensions/hooks.py (Diff revision 8)

Show all issues

Col: 60
 E251 unexpected spaces around keyword / parameter equals

reviewboard/extensions/hooks.py (Diff revision 8)
Show all issues
```
Col: 22
 E225 missing whitespace around operator
```
reviewboard/extensions/hooks.py (Diff revision 8)
Show all issues
```
Col: 5
 E303 too many blank lines (2)
```
reviewboard/reviews/views.py (Diff revision 8)
Show all issues
```
Col: 5
 E265 block comment should start with '# '
```

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

Show all issues

Col: 28
 E226 missing whitespace around arithmetic operator

Change Summary:

Reviewbot fixes.

Diff:

Revision 9 (+237 -7)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/index.rst~



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/index.rst~

reviewboard/extensions/hooks.py (Diff revision 9)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

user_infobox_10172015.png (Revision 1)

Show all issues

There's a mismatch in styles here. We should aim for consistency, or separate them out more by whitespace.

Also, the lines are very close together. Let's add some spacing between all lines.

Looks like you have some files in here (the documentation ending with a ~) that you probably didn't mean to have.

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 9)
Show all issues
```
"HTML"
```

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 9)

Show all issues

A couple things:

1) When showing examples like this, we want them in literal blocks.
2) I have no idea what this section is trying to say, so an extension author is going to be even more confused. Saying <html>...</html> suggests an entire HTML page will be in there.

It's better to use more concrete examples and to explain the structure in words.

reviewboard/extensions/hooks.py (Diff revision 9)
Show all issues
```
This shouldn't be here.
```

reviewboard/extensions/hooks.py (Diff revision 9)

Show all issues

These docs, just like the ones above, will be generated and made available on our site. This needs to be more explanatory, like above.

reviewboard/extensions/hooks.py (Diff revision 9)
Show all issues
```
No blank line here.
```

reviewboard/extensions/hooks.py (Diff revision 9)

Show all issues

This shouldn't take a list of sections. That's going to lock us into a particular design, and it's going to be hard to adapt this to any future changes.
I also don't think it's useful to take pre-defined strings. Infoboxes aren't going to show a single static string. They're going to need to compute things for the given user.
I'd suggest a different design for the hook:
1) Each UserInfoBoxHook should be about showing one single thing, not multiple things. Extensions can always instantiate more than one hook.

2) The constructor should take only two things: 1) A section indicator (represented by a constant on this class indicating the section name -- this would be mapped to a string like "header", etc.), and 2) an optional template name.

3) The render function below needs to take the user (since an extension will be basing info on the user), and should default to rendering the provided template with the request and user as context. However, the documentation should state that render can be overridden to provide custom rendering.
This should not take is_enabled, as it makes no sense to instantiate a hook and just keep it disabled.
We also need to rethink the sections. "header", "body", "footer" locks us into that model for the infobox, meaning we can't shuffle it later. For instance, say we want to make some of this appear side-by-side. "Footer" no longer makes sense there.
Instead, let's go with "user-identity" (names, e-mail, possibly department, etc.), and "extra-details". I think going with a third doesn't make a lot of sense right now.

reviewboard/extensions/hooks.py (Diff revision 9)
Show all issues
```
Missing a docstring.
No blank line here.
```
reviewboard/extensions/hooks.py (Diff revision 9)
Show all issues
```
Two blank lines here.
```

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

Show all issues

This is a third-party module.

CH

chronicleyu Oct. 22, 2015, 3:40 p.m.
```
So we can't use pytz?
```

brennie Oct. 22, 2015, 3:42 p.m.

No, it doesn't. However, we sort our imports into three groups:
```python

from future import unicode_literals  # Always.
import os  # Python standard library.
import pytz  # Third party modules
import reviewboard  # project imports.

CH

chronicleyu Oct. 22, 2015, 4 p.m.
```
Ok got it. Thanks.
```

reviewboard/reviews/views.py (Diff revision 9)
Show all issues
```
This must be inserted alphabetically.
```

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

Show all issues

This blank line must remain.

chronicleyu Oct. 20, 2015, 3:05 p.m.

Missed this one. Will fix it before the next post.

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

Show all issues

I'm going to want to see all this logic heavily documented, so every bit of it is clear.

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

Show all issues

What's all this?

chronicleyu Oct. 20, 2015, 3:05 p.m.

It's another method for calculating the time difference. Unlike the one above, it doesn't use divisions (which can be suboptimal), so I left it here commented out in case we want to use this method instead.

david Oct. 20, 2015, 3:08 p.m.
```
What is suboptimal about division?
```

chronicleyu Oct. 20, 2015, 3:16 p.m.

I may be overthinking. Was always told in assembly language classes that division operations take more clock cycles to complete. Would the same principle apply here?

david Oct. 20, 2015, 3:25 p.m.

It's true, a division instruction takes longer than, say, addition. However, we're operating at a much, much, much higher level here. Compared with the cost of querying a database that can live on a separate server, division is basically instantaneous. Even if you're talking about some small, tight, mathematically-intensive code, on modern CPUs it's probably not worth trying to optimize out your divisions (the long pipelines, vectorization, and speculative/out-of-order execution mean that the copiler and CPU microcode can usually do a better job than you can). It's only when writing code for simple, slow, basic microcontrollers where that kind of optimization makes sense.

As Donald Knuth said, "premature optimization is the root of all evil." What that means is that you need to write simple code first, and then measure it to see where the bottlenecks are. If you start out trying to optimize, you'll end up writing complex, unmaintainable code that probably optimizes the wrong thing.

chronicleyu Oct. 22, 2015, 3:49 p.m.

Gotcha. It makes sense now that you've put it this way.

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

Show all issues

<html> means the start of a document. We never want that, or we'll confuse browsers.

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

Show all issues

On a large deployment, this may produce thousands or tens of thousands of review requests.
Since we're just counting, we want to let the database handle this. We also don't need to re-check the user, since that's already part of the query. So:

review_requests_sent = ReviewRequest.objects.public(user=user).count()

chronicleyu Oct. 22, 2015, 3:38 p.m.

I found that

ReviewRequest.objects.public(user=user).count()

returns the count of all review requests visible to the user, not the review requests posted by the user his/herself. This is why I have to go into the result and manually find the ones where the submitter is the user.

david Oct. 22, 2015, 3:43 p.m.

We can still make this use the database, and should pass the request's user in to public so that we don't show them counts of things that aren't visible to them:

review_requests_sent = (
    ReviewRequest.objects
    .public(user=request.user)
    .filter(submitter=user)
    .count())

CH

chronicleyu Oct. 22, 2015, 3:53 p.m.
```
Ok, I'll try this. Thanks!
```

reviewboard/reviews/views.py (Diff revision 9)
Show all issues
```
Blank line between these.
```
reviewboard/reviews/views.py (Diff revision 9)
Show all issues
```
Missing a trailing comma.
```
reviewboard/templates/accounts/user_infobox.html (Diff revision 9)
Show all issues
```
These should be one statemnet.
```

reviewboard/templates/accounts/user_infobox.html (Diff revision 9)

Show all issues

Make sure all the content aligns properly with the current indentation level.

Change Summary:

Overhaul of the extension hook code and minor infobox UI improvements.

Summary:

-	User Infobox Improvements
+	WIP: User Infobox Improvements

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

~		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook.
	~	Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

~		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
	~	NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
	+	UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.

Testing Done:

		Hovering over a test account usernames on a local test server.
~		Unit testing for the extension hook.
	~	UPDATE (10/20/2015): Will have to re-test the hook after the architecture overhaul.

Groups:

+	students

Diff:

Revision 10 (+241 -10)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	2 more

Removed Files:

Added Files:

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

reviewboard/extensions/hooks.py (Diff revision 10)
Show all issues
```
 'format_html' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 10)
Show all issues
```
 'mark_safe' imported but unused
```
reviewboard/extensions/hooks.py (Diff revision 10)
Show all issues
```
Col: 39
 E231 missing whitespace after ','
```
reviewboard/extensions/hooks.py (Diff revision 10)
Show all issues
```
Col: 9
 E303 too many blank lines (2)
```
reviewboard/extensions/hooks.py (Diff revision 10)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

reviewboard/extensions/hooks.py (Diff revision 10)

Show all issues

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

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

Show all issues

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

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

Show all issues

Col: 13
 E123 closing bracket does not match indentation of opening bracket's line

reviewboard/reviews/views.py (Diff revision 10)
Show all issues
```
Col: 43
 E231 missing whitespace after ','
```

Change Summary:

Fixed ReviewBot errors which appeared after the previous post.

Diff:

Revision 11 (+241 -9)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

Change Summary:

Added documentation on extending the UserInfoBoxHook. Minor fixes to issues raised about the code in views.py.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
		UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
	+
	+	LATEST UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

Diff:

Revision 12 (+226 -9)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

reviewboard/reviews/views.py (Diff revision 12)
Show all issues
```
Col: 51
 E202 whitespace before ')'
```

Change Summary:

Removed WIP from title.

Summary:

-	WIP: User Infobox Improvements
+	User Infobox Improvements

Change Summary:

Fixed a whitespace issue in views.py from ReviewBot

Diff:

Revision 13 (+226 -9)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	2 more

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/ui/datagrids.less
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

To remove a file from your review request you can do:

git rm <filename>
git commit -m "Removed <filename>"
rbt post -r 7662


Also, you're going to want to add backup files to your global .gitignore or turn them off in your .vimrc. To do the former (a good idea), add the following to your ~/.gitconfig:

[core]
    excludesfile = ~/.gitignore_global


And then create a ~/.gitignore_global file with the following:

*~

CH

chronicleyu Oct. 26, 2015, 11:56 a.m.
```
Thanks!
```

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 13)
Show all issues
```
This should start with .. code-block:: python
```

reviewboard/extensions/hooks.py (Diff revision 13)

Show all issues

Docstrings should start with a 1-line summary, then a blank line, then additional paragraphs of explanation.

reviewboard/extensions/hooks.py (Diff revision 13)

Show all issues

We should check that template_name is a string, because otherwise we'll call render_to_string and pass it None. Alternatively, just get rid of the default value for the parameter to force people to pass it in.

reviewboard/extensions/hooks.py (Diff revision 13)

Show all issues

ExtensionHook doesn't define get_extra_context so the super call is superfluous and useless.

reviewboard/extensions/hooks.py (Diff revision 13)
Show all issues
```
Remove this line.
```

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

Show all issues

We should just include the <span> in the template, instead of passing it in like this.

chronicleyu Oct. 27, 2015, 11:12 a.m.

So, what you mean is to do this:
'clock_symbol': format_html("<span class='fa fa-clock-o'></span> ")
instead of:
clk_s = format_html("<span class='fa fa-clock-o'></span> ")

...

'clocl_symbol': clk_s
Is that correct?

david Oct. 27, 2015, 11:48 a.m.

No, I mean in the template, instead of {{clock_symbol}}, use <span class='fa fa-clock-o'></span>. You can then remove it entirely from this file.

CH

chronicleyu Oct. 28, 2015, 11:31 a.m.
```
Understood, thanks. I've fixed it.
```

reviewboard/reviews/views.py (Diff revision 13)
Show all issues
```
Please put this line back.
```

Change Summary:

Minor fixes to extension hook architecture and infobox UI. Changes to extension hook docstring in hooks.py.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
~		UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
~
	~	UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
	~	UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.
-		LATEST UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

Testing Done:

		Hovering over a test account usernames on a local test server.
~		UPDATE (10/20/2015): Will have to re-test the hook after the architecture overhaul.
	~	UPDATE (10/20/2015): Will have to re-test the hook after the architecture overhaul.
	+	UPDATE (10/28/2015): Tested the hook within the user_infobox function in views.py.

Diff:

Revision 14 (+227 -6)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

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

Change Summary:

Removed unused import from views.py

Diff:

Revision 15 (+226 -5)

Show changes

	docs/codebase/generating-documentation.rst~
	docs/codebase/index.rst
	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    reviewboard/templates/accounts/user_infobox.html
    docs/codebase/generating-documentation.rst~
    docs/manual/extending/extensions/hooks/index.rst
    reviewboard/static/rb/css/common.less
    docs/codebase/index.rst~
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 15)

Show all issues

This should line up with the c in code-block.

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 15)

Show all issues

This should line up with the c in code-block.

reviewboard/extensions/hooks.py (Diff revision 15)

Show all issues

Can you reformat this as follows?

        raise ValueError(
            'Invalid UserInfoBox section: %s; must be one of %s'
            % (section, ', '.join(UserInfoBoxHook.SECTION_OPTIONS))

This avoids the text wrapping. We also prefer %-formatting parameters to appear on the next line when the string is long enough to warrant it.

reviewboard/extensions/hooks.py (Diff revision 15)

Show all issues

Can you reformat this as:

return render_to_string(self.template_name,
                        RequestContext(request, context))


It's easier to read this way.

reviewboard/reviews/views.py (Diff revision 15)
Show all issues
```
Remove this line.
```
reviewboard/reviews/views.py (Diff revision 15)
Show all issues
```
This comment is unnecessary.
```

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

Show all issues

Single quotes. Also, this should be six.text_type, not str:

from django.utils import six


The reason we use this is that we use unicode literals everywhere and unicode != str. However, in Python 3, unicode was renamed to str and str was renamed to bytes. We use six.text_type so that when we eventually transition to Py3, we will have less headaches.

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

Show all issues

Can you put UserInfoBoxHook.render on one line? that way its clear there is not supposed to be a space between them.

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

Show all issues

This should be
'... extension: "',
extension.id, e, exc_info=True)


(I know some places use exc_info=1, but we aren't doing that moving forward.)

chronicleyu Oct. 28, 2015, 12:43 p.m.

Is that double quote a typo?
So should it look like below?
logging.error('Error when running UserInfoBoxHook.render '

                          'function in extension: ',

                          extension.id, e, exc_info=True)

Change Summary:

Removed the files which were accidentally added to the review request.

Diff:

Revision 16 (+205 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

Change Summary:

Removed unnecessary lines and reformatted wrapping text in views.py and hooks.py. Fixed indenting in extension hook documentation.

Diff:

Revision 17 (+204 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

reviewboard/extensions/hooks.py (Diff revision 17)

Show all issues

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

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 17)
Show all issues
```
This blank line isn't necessary when there's no docstring.
```

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 17)

Show all issues

I'm not sure what "custom parameters..." means here, and it's not something that users can change (since render chooses what to pass in). We should list user, request.

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 17)
Show all issues
```
get_extra_context should return a dict, not a str.
```

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

Show all issues

There are fractional time zones that aren't 30 minutes (for example, Kathmandu, Nepal is GMT +5:45). I don't think this will do the right thing for those.

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

Show all issues

We should pass the raw numbers in to the template and put the text there.

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

Show all issues

As is, this won't include any of the arguments because the first argument doesn't have any format specifiers. You can also simplify it using logging.exception:

logging.exception('Error when running UserInfoBoxHook.render in '
                  'extension %s', extension.id)

Change Summary:

Extension hook documentation changes. Also, hour difference calculation now rounds to the nearest quarter of an hour. Minor fixes to code in hooks.py, views.py and infobox template.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
		UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.
	+
	+	LATEST UPDATE (10/29/2015):
	+	* Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
	+	* Hour difference calculation in views.py now takes quarterly differences into account.

Diff:

Revision 18 (+220 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

reviewboard/reviews/views.py (Diff revision 18)
Show all issues
```
Col: 30
 E201 whitespace after '('
```
reviewboard/reviews/views.py (Diff revision 18)
Show all issues
```
 local variable 'e' is assigned to but never used
```

Change Summary:

ReviewBot fixes to views.py.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
		UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

~		LATEST UPDATE (10/29/2015):
~		* Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
~		* Hour difference calculation in views.py now takes quarterly differences into account.
	~	LATEST UPDATE (10/29/2015):
	~	- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
	~	- Hour difference calculation in views.py now takes quarterly differences into account.

Diff:

Revision 19 (+220 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

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

Show all issues

How about the following:

# We round the timezones
while minutes >= 10:
    if ahead:
        hr_diff += 0.25
    else:
        hr_diff -= 0.25

    minutes -= 15

This will have the same effect, but be much shorter.

Also, personally I'd much rather see 5:45 hours ahead of you instead of 5.75 hours ahead of you.

chronicleyu Oct. 29, 2015, 12:14 p.m.

If we're going with the '5:45 hours ahead of you' format, do we still want to round to the nearest 15 mins?

brennie Oct. 29, 2015, 12:21 p.m.
```
Lets leave that out for now.
```

david Oct. 29, 2015, 12:26 p.m.

Since time zones are essentially a political issue, I wouldn't put it past someone to create something even wackier. Let's show the computed number of minutes without rounding.

reviewboard/reviews/views.py (Diff revision 19)
Show all issues
```
elif
```

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

Show all issues

When interpolating only one variable, you don't need to use dict interpolation.

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

Show all issues

So string additions are slow. How about:

custom_info_parts = []

for hook in UserInfoBoxHooks.hooks:
    try:
        custom_info_parts.append(hook.render(user, request))
    except Exception:
        # ...

custom_info = ''.join(custom_info_parts)

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

Show all issues

We want to make sure that this isn't going to be escaped incorrectly. We should be checking if the result of hook.render is a SafeString or not. If not, it should be escaped so that we only deal with SafeStrings.

reviewboard/reviews/views.py (Diff revision 19)
Show all issues
```
extension.id on next line.
```

Change Summary:

Changes to hour difference display formatting and hook processing.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

~		NOTE: I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
	~	NOTE: (FIXED) I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
-		UPDATE (10/20/2015): I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
-		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

~		LATEST UPDATE (10/29/2015):
	~	UPDATE (10/20/2015): (FIXED) I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
	+
	+	UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.
	+
	+	UPDATE (10/29/2015 - A):
		- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
		- Hour difference calculation in views.py now takes quarterly differences into account.
	+
	+	UPDATE (10/29/2015 - B):
	+	- Hour difference display now shows "4:15 hours ahead" instead of "4.25 hours ahead" (attached screenshot updated).
	+	- Hook processing in views.py now uses a list for joining strings instead of string additions.

Diff:

Revision 20 (+175 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Removed Files:

Added Files:

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

user_infobox_10292015.png (Revision 1)

Show all issues

Now that we're showing this using a time notation, we can probably skip the word "hours" ("4:00 hours" seems weird, and "4:30 hours" seems weirder)

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

Show all issues

Can we move all of this computation to happen after the etag_if_none_match call? We don't want to do this if the user's browser already has it cached.

chronicleyu Oct. 31, 2015, 7:41 a.m.

Just to clarify, so everything from line 1612 to line 1651?

david Nov. 2, 2015, 10:38 a.m.
```
1612 to 1675 (all your additions)
```

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

Show all issues

I suspect we should show something else (or not show this at all) if request.user is the same as user. I know I'm in my own timezone!

chronicleyu Oct. 31, 2015, 7:41 a.m.

Actually, the UI is not showing my own timezone. But I need it to calculate the time difference.
So I'm using my_user_tz = pytz.timezone(request.user.get_profile().timezone) to get the datetime in

the next line: my_user_dt = utc_now.astimezone(my_user_tz).replace(tzinfo=utc), so that I can

get the timedelta between this datetime and the requested one: duration = requsr_dt - my_user_dt.

david Nov. 2, 2015, 10:38 a.m.

OK, if it doesn't show it for yourself, that's cool.

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

Show all issues

Can we localize this? We also should be able to skip the six.text_type call.

td_msg = _('Same local time as you (%s)') % requsr_tz

reviewboard/templates/accounts/user_infobox.html (Diff revision 20)

Show all issues

I know the existing text in here isn't localized, but we should mark these words (and the others in here) for translation. I think we should also put assigned/posted on separate lines.

chronicleyu Oct. 31, 2015, 7:41 a.m.

Is this correct:

<p class="user-stats">{% trans "Assigned: " %}{{users_stats.assigned}}</p>

<p class="user-stats">{% trans "Posted: " %}{{users_stats.posted}}</p>

david Nov. 2, 2015, 10:38 a.m.
```
Yes, that'll do.
```

Change Summary:

Text localization fixes and rearrangement of code in views.py. Minor infobox UI changes.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: (FIXED) I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.

		UPDATE (10/20/2015): (FIXED) I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.

		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

		UPDATE (10/29/2015 - A):
		- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
		- Hour difference calculation in views.py now takes quarterly differences into account.

		UPDATE (10/29/2015 - B):
		- Hour difference display now shows "4:15 hours ahead" instead of "4.25 hours ahead" (attached screenshot updated).
		- Hook processing in views.py now uses a list for joining strings instead of string additions.
	+
	+	LATEST UPDATE (11/02/2015):
	+	- Time difference calculation and review request statistics code now appears after the etag_if_none_match call.
	+	- UI and text localization changes to the user infobox.

Diff:

Revision 21 (+176 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Removed Files:

Added Files:

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 21)

Show all issues

If you do:

:py:class:`~reviewboard.extensions.hooks.UserInfoHook`

it will still link to the right class, but it will render as UserInfoHook.

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 21)
Show all issues
```
get_extra_context should return a

rst
:py:class:`dict`.
```
reviewboard/extensions/hooks.py (Diff revision 21)
Show all issues
```
Double backquotes
```
reviewboard/extensions/hooks.py (Diff revision 21)
Show all issues
```
Double backquotes
```
reviewboard/reviews/views.py (Diff revision 21)
Show all issues
```
No blank line here.
```

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

Show all issues

six.text_type, not unicode.
(from django.util import six)

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

Show all issues

How about:

review_requests_sent = (
    ReviewRequest.objects
    .public(user=request.user)
    .filter(submitter=user)
    .count()
)


Does this include ALL sent rrs or only the open ones?

CH

chronicleyu Nov. 14, 2015, 8:32 a.m.
```
All-time stats.
```

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

Show all issues

Why are you casting this to a string?

Does this include ALL assigned rrs or only open?

CH

chronicleyu Nov. 14, 2015, 8:32 a.m.
```
All-time stats.
```

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

Show all issues

Maybe we should we leave it up to the hook to return their strings as marked safe?

chronicleyu Nov. 14, 2015, 8:32 a.m.

We will potentially be calling mark_safe more than once if we do this. Would that be fine?

Change Summary:

Minor fixes to extension hook code and documentation, and to text type in views.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: (FIXED) I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.

		UPDATE (10/20/2015): (FIXED) I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.

		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

		UPDATE (10/29/2015 - A):
		- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
		- Hour difference calculation in views.py now takes quarterly differences into account.

		UPDATE (10/29/2015 - B):
		- Hour difference display now shows "4:15 hours ahead" instead of "4.25 hours ahead" (attached screenshot updated).
		- Hook processing in views.py now uses a list for joining strings instead of string additions.

~		LATEST UPDATE (11/02/2015):
	~	UPDATE (11/02/2015):
		- Time difference calculation and review request statistics code now appears after the etag_if_none_match call.
		- UI and text localization changes to the user infobox.
	+
	+	UPDATE (11/14/2015):
	+	- Minor fixes to extension hook code and documentation.
	+	- Change an occurrence of `unicode` to `six.text_type` in views.py.

Diff:

Revision 22 (+179 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

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

Show all issues

Col: 13
 E131 continuation line unaligned for hanging indent

Change Summary:

Hook rendering now does the mark_safe.

Description:

		Adds three pieces of information to the user info box:
		1) Hour difference with local computer
		2) User's review requests
		3) User's assigned requests

		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).

		NOTE: (FIXED) I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.

		UPDATE (10/20/2015): (FIXED) I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.

		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.

		UPDATE (10/29/2015 - A):
		- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
		- Hour difference calculation in views.py now takes quarterly differences into account.

		UPDATE (10/29/2015 - B):
		- Hour difference display now shows "4:15 hours ahead" instead of "4.25 hours ahead" (attached screenshot updated).
		- Hook processing in views.py now uses a list for joining strings instead of string additions.

		UPDATE (11/02/2015):
		- Time difference calculation and review request statistics code now appears after the etag_if_none_match call.
		- UI and text localization changes to the user infobox.

		UPDATE (11/14/2015):
		- Minor fixes to extension hook code and documentation.
		- Change an occurrence of `unicode` to `six.text_type` in views.py.
	+
	+	UPDATE (11/21/2015):
	+	- Moved the `mark_safe` for the extension hook to be done internally within the hook's render function.

Diff:

Revision 23 (+180 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

reviewboard/extensions/hooks.py (Diff revision 23)

Show all issues

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

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 23)
Show all issues
```
"A :py:class:`~reviewboard.extensions.hooks.UserInfoBoxHook` can be used..."
```
docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 23)
Show all issues
```
Blank space after comma
```

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 23)

Show all issues

"to return a :py:class:`dict`. The result of this function will be used, in addition to the request and user, for the template's rendering context."

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 23)
Show all issues
```
Should comment out the dots because its not valid python.
```
docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 23)
Show all issues
```
Provide an actual example.
```

reviewboard/extensions/hooks.py (Diff revision 23)

Show all issues

backquotes around the strings, e.g.

* ``'user-identity'``
* ``'extra-details'``

reviewboard/extensions/hooks.py (Diff revision 23)

Show all issues

If required, :py:meth:`get_extra_context` may be overridden ...

reviewboard/extensions/hooks.py (Diff revision 23)

Show all issues

If required, :py:meth:`get_extra_context` may be overridden ...

reviewboard/extensions/hooks.py (Diff revision 23)
Show all issues
```
Can we call this SECTIONS instead?
```

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

Show all issues

All comments should be complete sentences and should end in periods. This goes for all comments in this method.

reviewboard/reviews/views.py (Diff revision 23)
Show all issues
```
requser? How about target_user?
```

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

Show all issues

Instead of my_user, lets call it user or local_user

reviewboard/reviews/views.py (Diff revision 23)
Show all issues
```
This isn't a very useful comment.
```
reviewboard/reviews/views.py (Diff revision 23)
Show all issues
```
This is not a complete sentence.
```

reviewboard/templates/accounts/user_infobox.html (Diff revision 23)

Show all issues

We should probably wrap this in an {% if custom_info %}.
Also class="custom-info"

Change Summary:

Minor fixes to views.py, hooks.py, user_infobox.html, and extension hook documentation. Editted Description and Testing Done sections to be more consistent with commit messages.

Description:

~		Adds three pieces of information to the user info box:
~		1) Hour difference with local computer
	~	When hovering over a user's name, an info box appears showing their avatar/photo, name, e-mail address, and some other information. It's somewhat useful, but could be more useful.
	~	This change adds a couple new pieces of information:
-		2) User's review requests
-		3) User's assigned requests

~		Adds a UserInfoBoxHook to allow for more information to be added. Contains documentation on hook (WIP).
	~	1) Their timezone and how many hours ahead/behind you they are (to help determine if they'd be available at the moment).
	+	2) Some review request stats (how many review requests they have been assigned to, and how many they have had out for review).

~		NOTE: (FIXED) I did a git add -A and the index.rst and generating-documentation.rst got added into the review request. Will remove them in the next post.
	~	A new `UserInfoExtensionHook` is also created to provide addtional information for this box, should the need arise. That way, third-parties can integrate other services they use with this box. Documentation and example usage for the new hook are also provided.
-
-		UPDATE (10/20/2015): (FIXED) I can't get rid of the generating-documentation.rst file. If all fails, I will have to back everything up and create a new git branch.
-
-		UPDATE (10/22/2015): Top priority for this review request is feedback on the documentation.
-
-		UPDATE (10/29/2015 - A):
-		- Extension hook documentation: get_extra_context function now returns a dict, and render function now takes the correct params.
-		- Hour difference calculation in views.py now takes quarterly differences into account.
-
-		UPDATE (10/29/2015 - B):
-		- Hour difference display now shows "4:15 hours ahead" instead of "4.25 hours ahead" (attached screenshot updated).
-		- Hook processing in views.py now uses a list for joining strings instead of string additions.
-
-		UPDATE (11/02/2015):
-		- Time difference calculation and review request statistics code now appears after the etag_if_none_match call.
-		- UI and text localization changes to the user infobox.
-
-		UPDATE (11/14/2015):
-		- Minor fixes to extension hook code and documentation.
-		- Change an occurrence of `unicode` to `six.text_type` in views.py.
-
-		UPDATE (11/21/2015):
-		- Moved the `mark_safe` for the extension hook to be done internally within the hook's render function.

Testing Done:

~		Hovering over a test account usernames on a local test server.
~		UPDATE (10/20/2015): Will have to re-test the hook after the architecture overhaul.
	~	Moused over usernames to verify that the infobox appears with the new (and old) pieces of information displays correctly.
	~	Overrode `get_extra_context` to ensure that additional information added via the extension hook displays correctly.
-		UPDATE (10/28/2015): Tested the hook within the user_infobox function in views.py.

Diff:

Revision 24 (+179 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

reviewboard/extensions/hooks.py (Diff revision 24)

Show all issues

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

Change Summary:

Minor ReviewBot under-indent fix.

Diff:

Revision 25 (+179 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

reviewboard/extensions/hooks.py (Diff revision 25)

Show all issues

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

reviewboard/extensions/hooks.py (Diff revision 25)
Show all issues
```
We should call this SECTIONS
```

reviewboard/extensions/hooks.py (Diff revision 25)

Show all issues

We should indent this as:

return mark_safe(
    render_to_string(self.template_name,
                     RequestContext(request, context)))

reviewboard/reviews/views.py (Diff revision 25)
Show all issues
```
There's no need to cast this to a string.
```

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

Show all issues

We don't seem to be splitting this into the different sections.

chronicleyu Nov. 25, 2015, 12:11 p.m.

At the moment, all custom info goes in between the user's email and login/joined info. Do we want the custom info appearing in two different places depending on sections, or just have this code splitting it into different sections regardless?

Change Summary:

Added minor common.less CSS fix to custom info text on infobox. Added code in views.py to split custom context into different sections.

Diff:

Revision 26 (+189 -5)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

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

Show all issues

The etag needs to take into account the time difference and stats, and probably the extension hook stuff as well. You should talk to Christian about etags next week.

Change Summary:

Added entity tag updating for timezone info, RR statistics, custom rendering context. Updated extension hook documentation to reflect this change.

Description:

		When hovering over a user's name, an info box appears showing their avatar/photo, name, e-mail address, and some other information. It's somewhat useful, but could be more useful.
		This change adds a couple new pieces of information:

		1) Their timezone and how many hours ahead/behind you they are (to help determine if they'd be available at the moment).
		2) Some review request stats (how many review requests they have been assigned to, and how many they have had out for review).

~		A new `UserInfoExtensionHook` is also created to provide addtional information for this box, should the need arise. That way, third-parties can integrate other services they use with this box. Documentation and example usage for the new hook are also provided.
	~	A new `UserInfoExtensionHook` is also created to provide additional information, and associated entity tag data, for this box, should the need arise. That way, third-parties can integrate other services they use with this box. Documentation and example usage for the new hook are also provided.

Diff:

Revision 27 (+211 -8)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

Ship it!

Some minor issues, and then I'm happy with this landing.

reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 27)

Show all issues

According to the raw diff, you removed a newline from the last line. Please add it back.

reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary.
```

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

Show all issues

Can we just call this user_tz?

chronicleyu Dec. 7, 2015, 7:17 a.m.

Similarly, I have also changed local_user_dt to user_dt.

reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary
```

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

Show all issues

This isn't the entity tag. You have to call encode_etag on the result.

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

Show all issues

Can you just put one, larger comment above the timezone calculation stuff? We don't need comments every few lines -- the code is pretty self explanatory.

reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary.
```
reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary.
```
reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
No blank line here.
```
reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary.
```
reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Get.
```
reviewboard/reviews/views.py (Diff revision 27)
Show all issues
```
Unnecessary.
```

Change Summary:

Removed unnecessary blank lines and commenting, and renamed variables associated with the local user's timezone and datetime, in views.py. Readded a blank line in rb_extensions.py.

Diff:

Revision 28 (+201 -8)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

Tool: Pyflakes
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/reviews/views.py
    reviewboard/extensions/hooks.py

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

Change Summary:

Restored a missing s in return s in rb_extensions.py

Diff:

Revision 29 (+200 -7)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 29)
Show all issues
```
"custom rendering context"
```

docs/manual/extending/extensions/hooks/user-infobox-hook.rst (Diff revision 29)

Show all issues

"In this case, the get_etag_data should be overridden to return data that will be used to generate the entity tag for the infobox. This will help to ensure the infobox is cached correctly."

reviewboard/reviews/views.py (Diff revision 29)
Show all issues
```
"Get"
```
reviewboard/reviews/views.py (Diff revision 29)
Show all issues
```
Remove this.
```

Change Summary:

Revised extension hook documentation regarding get_etag_data in user-infobox-hook.rst. Minor fixes and blank line removals in views.py.

Diff:

Revision 30 (+200 -7)

Show changes

	docs/manual/extending/extensions/hooks/index.rst
	docs/manual/extending/extensions/hooks/user-infobox-hook.rst
	reviewboard/extensions/hooks.py
	reviewboard/reviews/views.py
	reviewboard/static/rb/css/common.less
	reviewboard/templates/accounts/user_infobox.html

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

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html



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

Ignored Files:
    docs/manual/extending/extensions/hooks/index.rst
    docs/manual/extending/extensions/hooks/user-infobox-hook.rst
    reviewboard/static/rb/css/common.less
    reviewboard/templates/accounts/user_infobox.html