Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 354753 - decorate user's comments in Gerrit reviews
Summary: decorate user's comments in Gerrit reviews
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P3 minor (vote)
Target Milestone: 0.9   Edit
Assignee: Kevin Sawicki CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2011-08-15 14:37 EDT by Kevin Sawicki CLA
Modified: 2012-03-21 11:36 EDT (History)
0 users

See Also:

Attachments
Patch to mark comments owned by current user (6.13 KB, patch)
2011-08-15 14:47 EDT, Kevin Sawicki CLA 		no flags Details | Diff
Screenshot post fix (33.47 KB, image/png)
2011-08-15 14:48 EDT, Kevin Sawicki CLA 		no flags Details
Second patch (10.34 KB, patch)
2011-08-15 17:15 EDT, Kevin Sawicki CLA 		no flags Details | Diff
Third patch (6.96 KB, patch)
2011-08-15 17:18 EDT, Kevin Sawicki CLA 		no flags Details | Diff
Fourth patch (6.23 KB, patch)
2011-08-15 17:20 EDT, Kevin Sawicki CLA 		no flags Details | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kevin Sawicki CLA 2011-08-15 14:37:01 EDT 
Currently all comments in a Gerrit review display with the blue person icon
while other connectors show your comments made by the current user with
a yellow person icon.
Comment 1 Steffen Pingel CLA 2011-08-15 14:46:38 EDT 
Thanks for pointing that out. We should look into fixing that.
Comment 2 Kevin Sawicki CLA 2011-08-15 14:47:31 EDT 
Created attachment 201511 [details]
Patch to mark comments owned by current user

Check repository account id against comment author id
Comment 3 Kevin Sawicki CLA 2011-08-15 14:48:18 EDT 
Created attachment 201512 [details]
Screenshot post fix
Comment 4 Kevin Sawicki CLA 2011-08-15 14:49:25 EDT 
I have attached a patch that caches the account id as a property on the task repository
and compares it with the author user id when iterating over comments.
Comment 5 Steffen Pingel CLA 2011-08-15 16:52:50 EDT 
Thanks for the patch. The idea looks good to me. I wonder though if we could use the account object that's already retrieved in GerritClient.getInfo()? 

I have a bit of a preference to store the account mapping as part of the configuration rather than the TaskRepository object. It seems fine to do it the way it is done in the patch for now though. Once we need to store additional fields we may want to reconsider.
Comment 6 Kevin Sawicki CLA 2011-08-15 16:57:41 EDT 
Would you be okay with making GerritClient.getAccount(IProgressMonitor) public instead of private?
Comment 7 Steffen Pingel CLA 2011-08-15 17:10:12 EDT 
(In reply to comment #6)
> Would you be okay with making GerritClient.getAccount(IProgressMonitor) public
> instead of private?

Sure, that seems fine.
Comment 8 Kevin Sawicki CLA 2011-08-15 17:15:26 EDT 
Created attachment 201526 [details]
Second patch

Calls GerritClient.getAccount directly
Comment 9 Kevin Sawicki CLA 2011-08-15 17:18:57 EDT 
Created attachment 201527 [details]
Third patch
Comment 10 Kevin Sawicki CLA 2011-08-15 17:20:53 EDT 
Created attachment 201528 [details]
Fourth patch
Comment 11 Steffen Pingel CLA 2011-08-16 09:19:29 EDT 
Thanks Kevin! I have committed the patch as 95e0a9427073528b83d5066fcb5ca0ec29a82c55.