Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(142)

Issue 5475061: Fix issue 292: Tooltips for developer names are confusing

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 1 month ago by techtonik
Modified:
12 years, 6 months ago
Reviewers:
Andi Albrecht
CC:
codereview-discuss_googlegroups.com
Visibility:
Public.

Description

http://code.google.com/p/rietveld/issues/detail?id=292

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+17 lines, -11 lines) Patch
M codereview/views.py View 2 chunks +15 lines, -10 lines 4 comments Download
M templates/user_popup.html View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 2
techtonik
14 years, 1 month ago (2011-12-10 10:43:47 UTC) #1
Andi Albrecht
14 years ago (2011-12-15 05:07:19 UTC) #2
http://codereview.appspot.com/5475061/diff/1/codereview/views.py
File codereview/views.py (right):

http://codereview.appspot.com/5475061/diff/1/codereview/views.py#newcode729
codereview/views.py:729: user email or account nickname, and sets
request.user_to_show
I'm not sure how we aligned multi-line docstring, especially when it's the first
line. But I'd either write this in a single line or aligning "user" at the
height of the first quotation mark.

http://codereview.appspot.com/5475061/diff/1/codereview/views.py#newcode3693
codereview/views.py:3693: num_issues_open = models.Issue.all()\
Personally I prefer to create filters by using multiple statements instead of
breaking a single statement in multiple lines by using backslashes. 

But maybe that's really just a personal thing :)

http://codereview.appspot.com/5475061/diff/1/codereview/views.py#newcode3701
codereview/views.py:3701: .filter('reviewers =', user.email())\
Do these changes require new indexes?

I wonder how we should handle private issues here. Should we exclude them from
this public stats? What do you think?

http://codereview.appspot.com/5475061/diff/1/codereview/views.py#newcode3706
codereview/views.py:3706: {'userinfo': user,
Why did you change this? Do we have any naming conflicts in the template?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b