Reviews/R4E/Design Discussion

Mylyn R4E Connector design notes Sebastien Dubois 14/09/2012 – version 1

Overview and Purpose

This documents outlines various proposals and notes that will help in the design and implementation of the Mylyn R4E Connector. The main goal of this connector is to integrate Mylyn and R4E functionality, in a similar way of what is done for the Mylyn Gerrit Connector. As such, the connector will handle the R4E elements as follows:

1. Review Groups and Design Rules will be handled in the Mylyn Task Repositories view
2. Reviews will be handled as tasks in the Mylyn task view.
3. Review properties, Participants, Global Anomalies, Review Items and Files will be

handled in the new Mylyn R4E Editor view.

1. Deltas, Selections, Anomalies and Comments will be handled in the various content editors as inline annotations

Concepts

R4E meta-data master copy: The main copy of the R4E meta-data, stored in the shared drive and accessible to all R4E users. R4E Mylyn Editor: The new view that will be displayed when a review task is opened.

Assumptions

It is assumed that:

1. The current R4E storage solution i.e. A file structure created within a shared drive will be the current storage solution used (at least initially). This storage space will hold the master copy of the R4E meta-data i.e. The meta-data that all users will see and refresh to.
2. The connector will interface with the Mylyn repository view and tasks view work flow as they are today. This means that the elements will be handled like any other corresponding Mylyn element. This also means that, unlike the current R4E Review Navigator View, the R4E meta-data used in the Connector will be locally available in the Mylyn workspace. So a merge/re-base mechanism and some synchronization/dirty state detection for the R4E meta-data stored as EMF model data will need to be implemented.
3. The R4E Review Navigator and the Mylyn R4E connector are independent from each other and can both be used separately to perform reviews. However, it is acceptable that both UI Clients (R4E Review Navigator and R4E Mylyn Connector UI) do not

display the same data at a given time, since the Review Navigation needs to be connected and will thus always be synchronized this the master data stored, whereas the R4E Mylyn connector will have its own local copy of the meta-data that will be submitted/merged with the master data by the user infrequently

1. As much as possible, the “looks and feel” of both UI clients should be consistent. This means that the same icons, decorators, labels, naming conventions etc. Should be re- used.
2. The R4E current code base should be re-used as much as possible. Common code should be made available in a common framework/libraries to avoid unnecessary code duplication.

UI Design Considerations (R4E Elements)

Review Groups

Presentation

The R4E Review Groups will be presented as repositories in the Mylyn Task Repositories view. The Review Group Properties (Name, Meta-data file path, Description, default Entry Criteria, Available Projects, Available Components and Associated Rule sets) should be available and editable in the repository properties dialog. There are 2 alternative ways the presentation could be achieved

Alternative 1: 1 Review Group = 1 Tasks Repository

A Review Group is displayed as a task repository. This means that multiple R4E repositories will coexist within the view. Pros:

1. Simpler Repository dialog content
2. 1-1 mapping is more intuitive

Cons:

1. Review Queries (see below) can only be done on 1 group at a time
2. Multiple Repositories can eventually become overwhelming

Alternative 2: All Review Group are under 1 “master” R4E task repository

Only 1 R4E task repository is created and all Review Groups are included within it. Pros:

1. Task repository content is minimized = 1 R4E repository only
2. Possibility to have Review queries across multiple Review Groups. Cons:
3. Repository properties more complex because multiple Review Group will be displayed. Some strategies can be implemented to deal with the added complexity e.g. Tabbed views, Dialog split between Review Group list at the top and Common properties for the currently selected group at the bottom....

We recommend alternative 2 for implementation.

Actions/Commands

Regardless of the alternative selected the following actions should be available

1. Creating a new Group
2. Modify Review Group properties
3. Submit modifications to the master data (including meta-data merge/re-base)

Rule Sets, Rule Areas, Rule Violations, Rules

Presentation

There are two alternatives for Rule Set presentation:

Alternative 1: Rule Sets as separate Task repositories

This involves having the rules sets are separate repositories like the reviews group above, and also with the same alternatives i.e. 1 repository for all rules sets or 1 repoper rule set. Pros:

1. Easier to visualize rule sets as separate entities

Cons:

Alternative 1: Rule Sets are included within Review Group repositories

This can only be done if alternative 2 is selected for Review Group above. It involves adding the rule sets directly to the R4E Review group properties e.g. As a separate section of the Group repository properties dialog Pros:

1. Very easy to apply rule sets to the review groups
2. To apply rule sets to review groups, repository cross references would be needed. Is this possible?

Cons:

1. Added complexity on the Review Group Task Repository properties dialog

We recommend alternative 2. Regardless of the alternative chosen, Rules sets, their children and their respective properties will be displayed in the task repository properties view

Commands

The following actions should be available:

1. Create Rule Set
2. Create Rule Area under a given rule set
3. Create Rule Violation under a given rule area
4. Create Rule under a given rule violations
5. Modify Rule properties
6. Submit rule modifications to the master data (including meta-data merge/re-base)

Reviews

Presentation

Reviews will be handled as tasks within the Mylyn Tasks view. As such they will re-use the current Mylyn functionality for tasks. The Create task wizard displayed when creating an R4E review task should include the same content as the current New Review... dialog in the R4E Review Navigator, and also should include a way to specify the parent Review Group to use for the new Review. Creating a new R4E review tasks will trigger to opening of a new instance of the R4E Mylyn editor, which will hold the data for the whole review, including the direct presentation of the Review properties, Review Items and File Contexts included. Review Queries can be performed to look for and filter out reviews based on specific criteria:

1. Review Group(s) (if alternative 2 is chosen for Review Groups see 3.1.1)
2. Review Type(s) Basic, Informal, Formal
3. Review State(s)
4. My Reviews (Reviews the current user is a participant of)
5. User Reviews ( Reviews a specific user is a participant of)

This should be done in a similar way the current Mylyn queries are currently done (e.g. Bugzilla queries)  Once the R4E Mylyn Editor is open. The review properties can be displayed in specific section(s) and can be modifiable as well in a similar way properties are displayed in the Properties view fro review in the R4E Review Navigator. Depending on the Review type (Basic, Informal, Formal), the Review properties displayed will be different. Currently in R4E, only 1 review can be open/active at any given time. The active review concept is very important and used widely in the code. From an R4E Mylyn Connector perspective, a review will be deemed active/open if it is currently the active task in the Mylyn tasks view.

Commands

1. Create Review (as a Mylyn tasks view Create Task action)
2. Modify Review Properties ( in the R4E Mylyn Editor)
3. Progress Review to its next state (in the R4E Mylyn Editor)
4. Regress Review to its previous state (in the R4E Mylyn Editor)
5. User Review Completed Indication (Placeholder only, this functionality is currently under implementation in R4E)
6. Generate BIRT report for the review(s) (either in the Mylyn Tasks view or in the R4E Mylyn Editor TBD)

Participants

Presentation

The Review participants should be included as a section the the R4E Mylyn Editor. Since there will be multiple participants within a specific review, they will need to be displayed as a list with a common property area where the properties for the currently selected participant could be displayed. The properties should be editable the same way they are in the Properties view of the R4E Review Navigator.

Commands

1. Add a participant to the Review (in the R4E Mylyn Editor)
2. Modify participant Properties (in the R4E Mylyn Editor)

Global Anomalies and Comments

Global anomalies are anomalies that are not specifically tied to a specific location in the review content, but are general review comments. As such, they are child to the review directly. They can have child comments as well

Presentation

The Global Anomalies will need to be displayed as a section within the R4E Mylyn Editor. They each will also need the ability to hold an arbitrary number of child comments. They could for instance be displayed a a tree structure within the section, with potential child comment under each and a common property section that will display the anomaly properties, in a similar way this is done for the properties view of the R4E Review Navigator.

Commands

1. Create a new Global anomaly (in the R4E Mylyn Editor)
2. Modify Global Anomaly Properties (either in the R4E Mylyn Editor or in a specific

dialog TBD)

1. Progress Global Anomaly to its next state (in the R4E Mylyn Editor)
2. Regress Global Anomaly to its previous state (in the R4E Mylyn Editor)
3. Add a comment to a specific Global Anomaly
4. Send Email Notification for the selected Global anomaly
5. Copy comment from a global anomaly
6. Paste comment in clipboard to a global anomaly

Review Items

A Review Item is a container of modified files that are related to each other e.g. Git Commit, SVN checking etc.

Presentation

Review Items are very similar to patch sets in Gerrit. As such they could be displayed in a similar way. The main difference is that Gerrit patch sets are a group a related commits, whereas R4E Review Items are arbitrary groups of files (i.e. Unrelated commits), or files that are manually included in the review (outside of any source control repository) Like Gerrit patch sets, they should be displayed in their own section of the R4E Mylyn Editor. Their properties, which will be different whether they are included as commits/check- ins or as manually selected review content, should also be displayed there. In the R4E Review Navigator, decorated icons are used to display the Review Items. This should also be the case here. A check mark decorator is added to the icon when the Review Item is marked as reviewed by the current user. Review Items are added by using the Find Review Items... command in any eclipse workspace explorer project (Commit Review Items), or by selecting a file in any explorer project or an arbitrary text range in a file open in a content editor (Resource Review Item). When adding a new Review Item to a Review, a dialog/wizard should be displayed to ask the user to which active review to add the Review Item to, but only if different reviews are open/active in the R4E review Navigator and in the Mylyn tasks view.

Commands

1. Mark/Unmark the Review Item as reviewed by the current user
2. Assign/Unassign the Review Item to specific participant(s)
3. Compare this review Item to another one included in the same review (New functionality not implemented in the current R4E)

File Contexts

In R4E, a File Context represents a duplet <base/ancestor file, target/current file> that is includes in a review as part of a Review Item. The target file is present if the file is new or modified for a commit Review Item or is a Resource Review Item, whereas the base file is present if the file is deleted or modified for a commit Review Item. These correspond to the files displayed on both side of the Eclipse Compare Editor

Presentation

The File Context will be displayed as a list of files (with icons) under a specific Review Item in the R4E Mylyn editor, in a similar way file are displayed in the Gerrit connector editor. As in the R4E Navigator, decorator should be added to the file icons if the file is new or deleted within this commit. The path and name of each file can be displayed as in the Gerrit connector Editor. It would also be nice to display a different file icons based on the file content (as in the Gerrit connector editor). While hovering over a file, a tooltip should display the base and target version ID , if available.

Commands

1. Double-clicking on a file should open the Eclipse compare editor with the target file on the right pane and the base on the left pane of the editor.
2. Mark/Unmark the Review Item as reviewed by the current user (In a Context Menu displayed by right-clicking on the file in the R4E Mylyn Editor)
3. Assign/Unassign the Review Item to specific participant(s) (In a Context Menu displayed by right-clicking on the file in the R4E Mylyn Editor)
4. Open the selected file in the single editor (In a Context Menu displayed by right- clicking on the file in the R4E Mylyn Editor)

Deltas and Selections

The Deltas correspond to changes in a specific file that is included in a commit Review Item. Selections are file/ranges in a file that in included manually as a Resource Review Item to the Review.

Presentation

Deltas and Selections should be shown as inline annotations within Eclipse contents compare editors that are opened from the R4E Mylyn Editor (by double-clicking on a file context element). This should be done in a similar way it is done in content editors opened from the R4E Review Navigator. The annotation pop-up will also display commands that could be used on the element. The R4E Annotation code (made common in the org.eclipse.mylyn.reviews.frame.ui plugin) should be re-used and extended for this purpose.

Commands

1. New Linked anomaly (from a command in the inline annotation pop-up toolbar)

Local Anomalies

Local anomalies are created on specific review content

Presentation

Local Anomalies should be shown as inline annotations within Eclipse contents compare editors that are opened from the R4E Mylyn Editor (by double-clicking on a file context element). This should be done in a similar way it is done in content editors opened from the R4E Review Navigator. The annotation pop-up will also display commands that could be used on the element. The R4E Annotation code (made common in the org.eclipse.mylyn.reviews.frame.ui plugin) should be re-used and extended for this purpose. When the compare editor is opened, an indicator should tell the user whether this editor was open from the Review Navigator (online) or the R4E Mylyn connector (offline) so that the user is not confused if the anomaly he enters does not show up in the R4E Review Navigator when it is entered offline (it will be shown when the offline content is published to the meta-data master copy

Commands

Create a new Local anomaly (in the specific contents editor as inline annotation)

1. Modify Local Anomaly Properties (from a command in the inline annotation pop-up toolbar)
2. Progress Global Anomaly to its next state (from a command in the inline annotation pop-up toolbar)
3. Regress Global Anomaly to its previous state (from a command in the inline annotation pop-up toolbar)
4. Send Email Notification for the selected Local anomaly (from a command in the inline annotation pop-up toolbar)

– Comments

Comments are added a child to anomalies to record follow-up comments

Presentation

Comments will be shown as child elements in the Content Editor Annotation pop-up display. See the current R4E inline Annotation functionality for an example

Commands

1. Add New comment to selected anomaly (from a command in the inline annotation pop-up toolbar)
2. Copy selected comment to clipboard (from a command in the inline annotation pop- up toolbar)
3. Paste clipboard comment to the selected anomaly (from a command in the inline annotation pop-up toolbar)

UI Design Considerations (Mylyn Connector views)

Mylyn Task Repositories View

The Mylyn Tasks repositories view will include Review Groups and Rule sets as described above in section 3.1 and 3.2. Additionally, LDAP and SMTP mail server configuration, currently included in the R4E preferences, could also be modified to be displayed as repositories (TBD)

Mylyn Tasks View

The Mylyn Tasks view will hold review in queries as stated above in section 3.3

Mylyn R4E Editor View

The Mylyn Editor view is a new view that will be created to display R4E review details. It should be structured in the same way as the Gerrit Connector Editor view, with main differences stated in sections 3.4 to 3.7 above.

Contents Editors

The various content compare editors will be opened by double-clicking on the file context elements displayed in the R4E Mylyn Editor. An indicator should be shown to tell users whether this was open from an online of offline element (see section 3.9.1 above). R4E modifies the Compare editor to navigate Anomaly annotations. This should also be available in editors opened from the R4E Mylyn Editor. Ideally, we would like to include the code navigability functions available in R4E. This means that the Editor input classes from R4E should be made available. This could also benefit the Gerrit Connector, which lacks this functionality currently. To display and use Deltas/Selections/Anomalies and Comments elements, the R4E Annotation functionality should be re-used

Synchronizing Review Meta-data

Currently every change made in the R4E EMF model form the UI Layer (Review Navigator or other R4E commands in editors) is immediately published to the master meta- data. So the EMF model data in RAM and the data written to the shared drive are always in sync for the a specific user. If another user updates data in the R4E master meta-data (on file), there is no mechanism to detect it. The user can however refresh the model data in RAM (and thus what he sees in the Review Navigator) at any time by command. Since the data is immediately committed, the potential for merge conflicts is minimal. A contention avoidance mechanism is implemented (checkout/check-in) to avoid having two users updating the master meta-data at the same time. However, since every user only holds the lock for a very brief period of time, and that the data is as much as possible separated by users on file, the likeliness of having such clashed is actually minimal. An unfortunate side effect of all this however is that R4E only supports an online (or connected mode). This means users must have access to the shared drive at all times when working with R4E and cannot work offline. On the other hand Mylyn has its own storage space where it holds a copy of the data it uses and only updates the master meta-data copy when the users publishes (or submits) the data changes. So effectively Mylyn always works in an offline mode all the time. It also has the capacity, when connected, to detect when the master meta-data has been changed (and thus not in sync with its own copy of the data) and a way to notify to users. This also means that, since the R4E master meta-data copy and the Mylyn own R4E local data copy can be different, that a merge/re-base function must be provided. So with the introduction of an offline mode for R4E, EMF core model data copies will exist as follows:

1. on RAM in the Review Navigator
2. serialized on shared file i.e. Master meta-data copy
3. serialized in the mylyn storage space
4. on RAM within the R4E mylyn connector

We should consider synchronization of the data between all these copies. Eventually, it is planned that it will be possible to also work offline using the Review Navigator, so a generic synchronization and merge/re-base solution should be provided so that is could be re-used when this functionality is implemented in the Review Navigator.

Synchronization between R4E Review Navigator and R4E master meta-data.

The model data copy in RAM is what is displayed to the users in the Review Navigator. As soon as the data in RAM changes, it is automatically written to the R4E master meta-data on file. When other users update the master meta-data, it will become out of sync with what the user sees in its Review Navigator view. However, since the change is done on file, there is no way to implemented a push notification that would let the user know about such changes. However, the user can at all time refresh its review navigator view to update its model data in RAM with the master meta-data copy on file. If the user tries to change data and the master copy was updated in the mean time, there is nothing now to handle any conflict, so the last update of a given element takes precedence and the older conflicting changes are lost. There is an Out of Sync exception that exists in the code to warn the user when this happens, but the functionality is not implemented as of today.

Handling meta-data within Mylyn R4E connector.

When a new entry is added to the Mylyn Taks Repositories view, a copy of the Review Group/Rule set data should be made in the mylyn local storage area. Likewise, when a query is made in the Mylyn tasks view and Review are imported, a copy of the Review data should be made as well to the local mylyn storage area. When the user works with Mylyn the UI reflect the model data that is in RAM. When the user saves the data, it gets written to the its local copy on file. As such there is never any possibility for any merge conflicts between these two model data copies.

Synchronization between Mylyn R4E Connector data and R4E master meta-data.

When the Mylyn user publishes (or submits) its changes, the changes in the data first gets written to the local copy on file, then they also get pushed to the master meta-data copy on the remote shared drive (and thus this drive needs to be accessible at this moment). Since any user, or even the same user using the R4E Review Navigator) could have updated the data before publishing, there are potential merge conflict in the data that can arise. The user can also refresh his own local copy of the data (on file and in RAM together). When refreshing the data, changes that were done on the master meta-data copy can also conflict with the changes done locally by the user on the Mylyn R4E connector and not yet published. Since that, unlike the R4E Review Navigator, multiple data changes could be done locally before being published/written to the master copy, there is a need to modify the contention avoidance mechanism in R4E core to have a more coarse granularity in locking files at write time. For example, we can lock the whole model from the Review level all the way down at once, serialize all the changes on file, then release the lock at once. Instead of locking every single element write separately.

Synchronization between R4E Review Navigator and Mylyn R4E connector.

Since the Review Navigator and the Mylyn R4E connectors can both be considered as two different UI clients of the same core R4E data (EMF model), there is no need to have them talk to each other. They will both synchronize with the master data copy directly.

R4E meta-data merge/re-base functionality and dirty state detection (Mylyn R4E connector meta-data publishing).

Synchronizing the UI clients (Review Navigator and Mylyn R4E connectors) implies that there is a need to implement a mechanism that could handle potential merge conflicts. There is also a need to implement a polling mechanism that will look at the master meta-data copy (when accessible) periodically, detect changes in the master data and notify the user of this. This could be done generically so that this function can be re-use for all UI clients (Review Navigator and R4E Mylyn connector. Upon publishing changes, if it was previously detected that there were changes to the master data, the submission will be rejected and the user will have to first refresh its local data (which will trigger a re-base of its local data) before submitting changes. The master data should also be polled immediately before trying to publish the user changes to verify that there were no recent undetected changes. If so the submission should be rejected as stated above. Take note that Mylyn already works in a similar fashion when interfacing with other repositories e.g. Bugzilla, Gerrit. So that same behaviour should be seen by the user. Re-basing the local data to the master copy involves traversing the whole model data structures and comparing each element/properties of the current master copy with the elements of the local copy and finding the differences (two-way compare), or to compare both with their common ancestor (three-way compare). To simply the implementation, a two-way compare mechanism should be used:

1. element properties in the local copy should overwrite any value in the master copy.
2. new elements in the local copies should be added to the master copy.
3. new elements in the master copy not in the local copy should be preserved.
4. ]It is not currently possible to remove elements from the R4E model so the removal case should not be considered.

Re-basing should be done in the UI clients. Once the re-base is done, the master meta-data copy can just be completely replaced with the re-based local copy.

Other Considerations

Automated Time tracking in R4E

In the R4E participant element, it is now possible to track the time as user spends working on reviews. Currently the time has to be manually entered by the user. With the mylyn integration, we can take advantage of the Mylyn automatic time tracking functions to update the participant properties with this info. When publishing new changes to the R4E master meta-data, the time tracking properties of the R4E participant should be updated automatically with the new time tacking information from Mylyn.

Adding due date for Reviews to the R4E model.

Since mylyn has the concept of due dates for tasks, a new due date property should be added to the R4E Review EMF model. This should be done at the frame core level, not on the derived R4E model.

Synchronizing Task Repositories and R4E Preferences

At least initially it will be possible to link Review Groups and Rules Sets either from the R4E Preferences, or from the Mylyn Task Repositories view. The Review Group data seen on both should be consistent i.e. Changes in the R4E Preferences should be reflected on the Mylyn Tasks Repositories View and vice-versa.

Use case 1: Linking existing Groups/Rulesets

When a new entry is created to link review group/ruleset in the Mylyn Task repositories view, the R4E preferences, the preference store for R4E should be updated to add this entry. Likewise, when a new Review Group/Ruleset link is added in the preferences, the Mylyn R4E connector should be used to add the information to the Mylyn Task repositories view. This is all done locally, so there is no change in the remote R4E master meta-data.

Use case 2: Creating new Groups/Rulesets from R4E Review Navigator

When new Groups/Rule sets are created from the R4E Review Navigator, the R4E remote master meta-data copy is immediately updated. The Mylyn Task repositories view information will then be out of sync. The new group thus created will automatically be added to the local R4E preferences, so a notification can be sent to the Mylyn R4E connector that will immediately update the Mylyn Task repositories view information.

Use case 3: Creating new Groups/Rulesets from the Mylyn Task repositories view.

This is not supported and should not be considered for now.

Other Open issues

1. Do we need a custom ID for each model element in the R4E core model to help with the Merging re basing? Right now we do not have such Ids.
2. Question: How does mylyn currently handle conflicting changes for other connectors e.g. Bugzilla, Gerrit?

Feature priority

Mandatory for initial release

Initial release should be focused on functionality needed to use the Connector in a reviewer role.

1. Review Group handling (including sync with R4E preferences)
2. Review Handling Queries
3. Review Handling in R4E Mylyn Editor
4. Participant Handling
5. Global Anomaly and Comment Handling (except copy/paste commands)
6. Review Item Handling (except Assign/Unassign commands)
7. File Context Handling (except Assign/Unassign and Open in Single Editor commands)
8. Delta/Selection Handling
9. Local Anomaly Handling
10. Comment Handling (except copy/paste commands)
11. Synchronization/Re-basing between mylyn local data copy and master copy
12. Master data polling and dirty state detection

Lower priority (subsequent releases)

1. Rule Set handling
2. Global Anomaly Handling (copy/paste commands)
3. Review Item Handling (Assign/Unassign commands)
4. File Context Handling (Assign/Unassign and Open in Single Editor commands)
5. Postponed Anomalies
6. Comment Handling (copy/paste commands)