I've simply allowed non-owner to change the reviewers list. Not sure if this is
desirable. But currently non-owners can already make comments to any issue, and
I think it is OK for anyone to register interests in an issue by adding herself
to the reviewer list.

I've thought about this and I agree that this is the way to go. There's really
no need to have separate Reviewers and CC lists, even though Mondrian does it
that way.

I have two nits and one suggestion for an enhancement.

http://codereview.appspot.com/776/diff/1/3
File rietveld/views.py (right):

http://codereview.appspot.com/776/diff/1/3#newcode949
Line 949: 'reviewers': ', '.join(issue.reviewers),
I would suggest one more addition: add the current user's email to the reviewers
if it's not already there (and this is not the issue owner).

http://codereview.appspot.com/776/diff/1/3#newcode965
Line 965: issue.reviewers = reviewers
I'd move this one line downfor symmetry with the 'if' clause (set subject first,
set issue attributes second).

http://codereview.appspot.com/776/diff/1/3#newcode1250
Line 1250: 
Don't add an extra blank line at the end.

I've made your suggested changes. And made a further change in _get_reviewers to
remove duplicates in the reviewers list.

http://codereview.appspot.com/776/diff/1/3
File rietveld/views.py (right):

http://codereview.appspot.com/776/diff/1/3#newcode949
Line 949: 'reviewers': ', '.join(issue.reviewers),
Good suggestion, I've added that. Just another thought: should we use nickname
when displaying and allow users enter either email or nickname? That would make
the list shorter in general.

http://codereview.appspot.com/776/diff/1/3#newcode965
Line 965: issue.reviewers = reviewers
On 2008/05/10 15:44:24, GvR wrote:
> I'd move this one line downfor symmetry with the 'if' clause (set subject
first,
> set issue attributes second).

Done.

http://codereview.appspot.com/776/diff/1/3#newcode1250
Line 1250: 
On 2008/05/10 15:44:24, GvR wrote:
> Don't add an extra blank line at the end.

Done.

http://codereview.appspot.com/776/diff/101/122
File rietveld/views.py (right):

http://codereview.appspot.com/776/diff/101/122#newcode30
Line 30: import sets
This is Python 2.5, it has a built-in set type. (Even 2.4 does :-)

http://codereview.appspot.com/776/diff/101/122#newcode591
Line 591: return list(reviewers)
I'd prefer to keep reviewers in the order that they were entered. The list is
never long enough to worry about quadratic behavior.

http://codereview.appspot.com/776/diff/101/122#newcode950
Line 950: in issue.reviewers):
You can use 'not in' for the latter test.

http://codereview.appspot.com/776/diff/101/122#newcode951
Line 951: issue.reviewers.append(request.user.email())
I wouldn't assign to issue.reviewers here, since you're not actually going to
write it back.  Use a local variable that's a copy of issue.reviewers, and pass
that to the form_class() call below.

Done. :)

Very close!

http://codereview.appspot.com/776/diff/125/43
File rietveld/views.py (right):

http://codereview.appspot.com/776/diff/125/43#newcode189
Line 189: send_mail = forms.BooleanField()
The order of the fields should be the same as in PublishForm -- i.e. send_mail
should come *after* reviewers.

http://codereview.appspot.com/776/diff/125/43#newcode1013
Line 1013: everyone = addressees + [my_email]
What would you think of *not* adding my_email in this branch? That way you could
opt out from receiving mail if you explicitly remove yourself from the
reviewers.

http://codereview.appspot.com/776/diff/125/43
File rietveld/views.py (right):

http://codereview.appspot.com/776/diff/125/43#newcode1013
Line 1013: everyone = addressees + [my_email]
Make sense. It would be confusing if this person receives an email this time but
not the subsequent conversations.

Looks good!

You can check it in yourself, I've added you to the project.

JiaYao, this is now live!