Bug 477635 - [UCOSP] contribute reviews section to task editor showing associated reviews
Summary: [UCOSP] contribute reviews section to task editor showing associated reviews
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows 7
: P3 enhancement (vote)
Target Milestone: 2.10   Edit
Assignee: Blaine Lewis CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed, noteworthy
Depends on: 482534
Blocks: 476925
  Show dependency tree
 
Reported: 2015-09-16 17:42 EDT by Blaine Lewis CLA
Modified: 2016-04-15 16:51 EDT (History)
0 users

See Also:

Attachments
mylyn/context/zip (3.83 KB, application/octet-stream)
2015-09-19 18:29 EDT, Sam Davis CLA 		no flags Details
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 Blaine Lewis CLA 2015-09-16 17:42:30 EDT 
contribute section only if task has associated reviews
display summary of incoming changes for each review in reviews section
display CR and V columns like the Gerrit dashboard view
add task list tooltip to reviews section
Comment 1 Eclipse Genie CLA 2015-09-18 13:48:43 EDT 
New Gerrit change created: https://git.eclipse.org/r/56268
Comment 2 Eclipse Genie CLA 2015-09-18 13:54:45 EDT 
New Gerrit change created: https://git.eclipse.org/r/56269
Comment 3 Eclipse Genie CLA 2015-09-18 14:26:04 EDT 
New Gerrit change created: https://git.eclipse.org/r/56275
Comment 4 Eclipse Genie CLA 2015-09-19 12:38:35 EDT 
New Gerrit change created: https://git.eclipse.org/r/56300
Comment 5 Sam Davis CLA 2015-09-19 18:29:43 EDT 
For the CR and V columns, you can copy the images from org.eclipse.mylyn.gerrit.dashboard.ui into /org.eclipse.mylyn.reviews.ui/icons and add them to ReviewsImages. Have a look at ReviewTableLabelProvider (see attached context) for how the Gerrit dashboard view uses them.
Comment 6 Sam Davis CLA 2015-09-19 18:29:50 EDT 
Created attachment 256697 [details]
mylyn/context/zip
Comment 7 Eclipse Genie CLA 2015-09-20 13:48:27 EDT 
New Gerrit change created: https://git.eclipse.org/r/56316
Comment 8 Sam Davis CLA 2015-10-06 17:11:43 EDT 
We should either load the task-review mapping synchronously for a given task when opening it, or keep the entire mapping (for all tasks) in memory all the time. The latter might have some advantages since we need to update it every time we get an incoming. Blaine I think you were looking at AbstractExternalizationParticipant before; it might make sense to take advantage of that so that loading and saving happens at the same time the task list is loaded and saved.

If you've already got it working the first way, maybe we can give it a try and consider changing it if the performance is a problem.
Comment 9 Blaine Lewis CLA 2015-10-06 22:09:09 EDT 
My current implementation keeps task-review mappings in memory, only serializing a single mapping when it changes and deserializing all at startup.

Using an AbstractExternalizationParticipant will assist in this but it creates a new issue: AbstractExternalizationParticipant's are a part of the TasksUiPlugin and this makes Tasks dependent on a review component. 
It also doesn't actually solve the race condition that occurs when an editor is open showing a task with reviews associated because AbstractExternalizationParticipant's execute asynchronously too. 

Is there a way I can force a Job to execute and join the current thread? They have a join method, but that only works if they are in the running state, not the waiting state.
Comment 10 Blaine Lewis CLA 2015-10-06 22:13:38 EDT 
I think that AbstractExternalizationParticipant is the right way to go because then the ExternalizationParticipantManager would have more control over the mappings and they'll act more consistently.
Comment 11 Blaine Lewis CLA 2015-10-07 16:34:16 EDT 
I decided on another method. The serialization I implemented didn't quite match what the AbstractExternalizationParticipant's did. To fix the times where the deserialization isn't complete when a mapping is requested, we manually load the mappings for that task.
Comment 12 Eclipse Genie CLA 2015-10-13 06:52:10 EDT 
New Gerrit change created: https://git.eclipse.org/r/58056
Comment 15 Sam Davis CLA 2015-12-10 14:35:44 EST 
.
Comment 16 Eclipse Genie CLA 2016-02-09 16:03:58 EST 
New Gerrit change created: https://git.eclipse.org/r/66247