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

Issue 4996047: Show approval status (green background for LGTMs) on Issues page

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 4 months ago by paul.rademacher
Modified:
14 years, 2 months ago
CC:
codereview-discuss_googlegroups.com
Visibility:
Public.

Description

This change displays the "approved" status for an issue, by coloring the issue green on the main Issues page. Before this, approval (an "lgtm" in a reviewer message) was only displayed on the message header inside each issue, but approval state could not be seen from the main Issues page. Demo at http://codereview-rademacher-demo.appspot.com/

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+9 lines, -1 line) Patch
M codereview/models.py View 1 chunk +1 line, -0 lines 0 comments Download
M codereview/views.py View 2 chunks +7 lines, -0 lines 1 comment Download
M templates/issue_row.html View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 9
paul.rademacher
14 years, 4 months ago (2011-09-10 20:52:55 UTC) #1
techtonik
Some projects (Chromium?) require at least 3 independent LGTMs before issue can be deemed approved, ...
14 years, 4 months ago (2011-09-10 21:57:19 UTC) #2
Andi Albrecht
I think this change is ok. We don't have a feature request for more advanced ...
14 years, 4 months ago (2011-09-11 07:36:27 UTC) #3
techtonik
On 2011/09/11 07:36:27, Andi Albrecht wrote: > I think this change is ok. We don't ...
14 years, 4 months ago (2011-09-11 08:14:04 UTC) #4
Andi Albrecht
On Sunday, September 11, 2011, <techtonik@gmail.com> wrote: > On 2011/09/11 07:36:27, Andi Albrecht wrote: >> ...
14 years, 4 months ago (2011-09-11 09:49:30 UTC) #5
techtonik
On 2011/09/11 09:49:30, Andi Albrecht wrote: > Ok. Do you have a suggestion how to ...
14 years, 4 months ago (2011-09-11 13:06:16 UTC) #6
paul.rademacher
Thanks for the discussion, guys. I'll send a revised patchset with: - issue.approvals: list of ...
14 years, 4 months ago (2011-09-12 08:18:46 UTC) #7
techtonik
On 2011/09/12 08:18:46, paul.rademacher wrote: > Thanks for the discussion, guys. I'll send a revised ...
14 years, 4 months ago (2011-09-15 09:35:01 UTC) #8
techtonik
14 years, 2 months ago (2011-10-25 20:59:39 UTC) #9
On 2011/09/12 08:18:46, paul.rademacher wrote:
> Thanks for the discussion, guys.  I'll send a revised patchset with:
> 
>    - issue.approvals: list of emails instead of a single "approved" boolean

Hi, Paul. How is the progress? Do you experience any troubles with that part?

>    - Exposure of approvals email list in API
>    - Possibly displaying the approval emails on the issues page (though
>    that'd be an extra column and might crowd the UI).  Just a thought.

This stuff can be added later.

>    - A response to Andi's comment on having to update issue in two places.
Sign in to reply to this message.

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