Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 354753

Summary:

decorate user's comments in Gerrit reviews

Product:

z_Archived

Reporter:

Kevin Sawicki <kevin>

Component:

Mylyn

Assignee:

Kevin Sawicki <kevin>

Status:

RESOLVED FIXED

QA Contact:

Severity:

minor

Priority:

Keywords:

contributed

Version:

unspecified

Target Milestone:

0.9

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Patch to mark comments owned by current user	none
Screenshot post fix	none
Second patch	none
Third patch	none
Fourth patch	none

Description Kevin Sawicki

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

2011-08-15 14:46:38 EDT

Thanks for pointing that out. We should look into fixing that.

Comment 2 Kevin Sawicki

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

2011-08-15 14:48:18 EDT

Created attachment 201512 [details]
Screenshot post fix

Comment 4 Kevin Sawicki

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

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

2011-08-15 16:57:41 EDT

Would you be okay with making GerritClient.getAccount(IProgressMonitor) public instead of private?

Comment 7 Steffen Pingel

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

2011-08-15 17:15:26 EDT

Created attachment 201526 [details]
Second patch

Calls GerritClient.getAccount directly

Comment 9 Kevin Sawicki

2011-08-15 17:18:57 EDT

Created attachment 201527 [details]
Third patch

Comment 10 Kevin Sawicki

2011-08-15 17:20:53 EDT

Created attachment 201528 [details]
Fourth patch

Comment 11 Steffen Pingel

2011-08-16 09:19:29 EDT

Thanks Kevin! I have committed the patch as 95e0a9427073528b83d5066fcb5ca0ec29a82c55.