Review Request #2419 — Created June 23, 2011 and submitted
I noticed that after I'd created a review with some issues, when I went and viewed my comments in the diff viewer, the buttons for dropping / fixing the issues were visible to me, even though I was not the review requester. This patch fixes that bug, as well as putting the issue status message in the inline comments. Not sure how that one slipped by.
Manual testing.
| Description | From | Last Updated |
|---|---|---|
|
Indentation is wrong. |
chipx86
|
|
|
We don't deal with scoping issues, so no need for self anymore. |
chipx86
|
|
|
You can return this. |
chipx86
|
|
|
First line of a comment should be blank. So: /* * Wraps an ... */ |
chipx86
|
|
|
Doesn't look like self is needed, since we never deal with scoping issues from callbacks. You can just use this … |
chipx86
|
|
|
Can just use this. |
chipx86
|
Thanks for the review! Instead of rendering the buttons and then hiding them, we now only render buttons on the inline comments if the issues are interactive.
Revision 2 (+32 -7)
Doesn't look like self is needed, since we never deal with scoping issues from callbacks. You can just use this below.
Is this actually replacing issue with some new element? If not, we can just do issue.issueButtons().
Same question here. Though, in this case, I don't see where issue is even used anymore. So I think we can remove the assignment.
Thanks for the review - updated with style changes. While I was patching this, I also noticed that the issue state for an inline comment wasn't being persisted on closing and re-opening the comment window, so I've fixed that by registering a simple callback with the issue manager.
Revision 3 (+38 -8)
