Bug 297437 - Support for patch review through Mylyn for Bugzilla
Summary: Support for patch review through Mylyn for Bugzilla
Status: RESOLVED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: Mylyn (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.4   Edit
Assignee: Frank Becker CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-10 02:01 EST by Deepak Kumar CLA
Modified: 2010-02-11 14:51 EST (History)
2 users (show)

See Also:


Attachments
commited patch (8.46 KB, patch)
2009-12-20 16:57 EST, Frank Becker CLA
eclipse: review?
Details | Diff
mylyn/context/zip (12.93 KB, application/octet-stream)
2009-12-20 16:57 EST, Frank Becker CLA
no flags Details
reverted patch (8.47 KB, patch)
2009-12-21 16:34 EST, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (10.68 KB, application/octet-stream)
2009-12-21 16:34 EST, Frank Becker CLA
no flags Details
screen shot (34.84 KB, image/png)
2010-01-11 13:57 EST, Steffen Pingel CLA
no flags Details
desired functionality screen shot (101.16 KB, patch)
2010-01-14 07:11 EST, Deepak Kumar CLA
deepaksrivastavaz: review?
Details | Diff
screen shot of new UI (70.17 KB, image/tiff)
2010-01-27 14:38 EST, Frank Becker CLA
no flags Details
patch for new UI (7.62 KB, patch)
2010-01-27 14:51 EST, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (21.41 KB, application/octet-stream)
2010-01-27 14:51 EST, Frank Becker CLA
no flags Details
screenshot (86.78 KB, image/tiff)
2010-02-01 17:23 EST, Frank Becker CLA
no flags Details
patch (19.76 KB, patch)
2010-02-03 15:20 EST, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (18.79 KB, application/octet-stream)
2010-02-03 15:21 EST, Frank Becker CLA
no flags Details
commited patch (6.83 KB, patch)
2010-02-11 14:49 EST, Frank Becker CLA
no flags Details | Diff
mylyn/context/zip (19.83 KB, application/octet-stream)
2010-02-11 14:50 EST, Frank Becker CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Deepak Kumar CLA 2009-12-10 02:01:09 EST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.5) Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)
Build Identifier: 20090920-1017

Currently Mylyn3.3.0v20091015-0500-e3x do not have support for asking patch review to peers or do a patch review while attaching the patch to Bugzilla.

There should be a feature that should have the following support:
1. On "Add Attachment" dialog box while attaching patch there should be fields asking for the person who should review the patch.
2. The reviewer should be listed in Bug Window -> Attachments section so that we are aware of who is about to review my patch.
3. If I have been asked for a review then Bug Window -> Attachments section should have another option/column for setting the status(r+, r- etc) of patch being reviewed prior to submit.

Above all need to update the Bugzilla fields values accordingly but might need to honor the current bug state i.e.; we can have optional configuration that says whether we are strict about if a bug is in Resolved-Fixed/Closed state then do we really want to add the patch as attachments to the bug with/without changing its state.

--Deepak

Reproducible: Always
Comment 1 Frank Becker CLA 2009-12-20 16:57:23 EST
Created attachment 154859 [details]
commited patch
Comment 2 Frank Becker CLA 2009-12-20 16:57:29 EST
Created attachment 154860 [details]
mylyn/context/zip
Comment 3 Frank Becker CLA 2009-12-20 16:58:32 EST
Steffen, Rob,

please review the commited patch.
Comment 4 Frank Becker CLA 2009-12-21 16:34:29 EST
Created attachment 154903 [details]
reverted patch

Sorry I have to do some cleanup and first want to wait until the review is finished.
Comment 5 Frank Becker CLA 2009-12-21 16:34:32 EST
Created attachment 154904 [details]
mylyn/context/zip
Comment 6 Steffen Pingel CLA 2010-01-11 13:57:47 EST
Created attachment 155782 [details]
screen shot
Comment 7 Deepak Kumar CLA 2010-01-14 06:35:59 EST
But Steffen I can't see any support for field to ask for peer review in the attachment, is this going under "Flags" sub-tab or elsewhere?

--Deepak
Comment 8 Deepak Kumar CLA 2010-01-14 07:11:26 EST
Created attachment 156086 [details]
desired functionality screen shot

Attaching screen shot of UI aspect of desired functionality.

--Deepak
Comment 9 Frank Becker CLA 2010-01-14 14:42:19 EST
(In reply to comment #7)
> But Steffen I can't see any support for field to ask for peer review in the
> attachment, is this going under "Flags" sub-tab or elsewhere?
> 
> --Deepak

Yes all flags are now under the Flags tab.
Comment 10 Steffen Pingel CLA 2010-01-14 14:48:34 EST
Frank, what is minimum Bugzilla version required for flag support? If it isn't already this should be documented in the Mylyn FAQ somewhere.
Comment 11 Frank Becker CLA 2010-01-14 15:03:16 EST
(In reply to comment #10)
> Frank, what is minimum Bugzilla version required for flag support? If it isn't
> already this should be documented in the Mylyn FAQ somewhere.

Steffen,

the flag support was add for Bugzilla 3.2 (see https://bugzilla.mozilla.org/show_bug.cgi?id=465219).
Comment 12 Robert Elves CLA 2010-01-14 15:52:53 EST
Frank, the ui for attachment attributes was discussed on today's call as well and for the same reasons cited in comment#6 on bug#290197, lets try using the expandable sections similar to the repository settings page to hold these attachment attributes instead of tabs.
Comment 13 Frank Becker CLA 2010-01-27 14:38:02 EST
Created attachment 157454 [details]
screen shot of new UI
Comment 14 Frank Becker CLA 2010-01-27 14:51:04 EST
Created attachment 157456 [details]
patch for new UI
Comment 15 Frank Becker CLA 2010-01-27 14:51:06 EST
Created attachment 157457 [details]
mylyn/context/zip
Comment 16 Robert Elves CLA 2010-01-28 11:31:29 EST
Great Frank. Added ui review to today's meeting agenda.
Comment 17 Robert Elves CLA 2010-01-28 13:49:44 EST
See comment regarding scrolled pane on bug#272207.  If you could get these two working consistently with the scrolled pane that would be ideal.
Comment 18 Steffen Pingel CLA 2010-01-28 14:19:12 EST
I agree that it's important that we keep consistency with the platform and use the standard control design (not the flat design) in dialogs.

Frank, it would be great if you could come up with a reusable implementation for a pane with expandable sections that shows a vertical scrollbar on demand. The repository settings dialog has the same requirement: bug 253142: [api] provide scrollbar to task repository properties page.

Another requirement that we discussed on the call:

* Add a "Run in background" checkbox which should be checked by default to make the attachment details dialog consistent with the attachment wizard. The implementation should be shared with the attachment wizard to the extend feasible.
Comment 19 Frank Becker CLA 2010-02-01 17:23:31 EST
Created attachment 157849 [details]
screenshot

changed UI with scroller
Comment 20 Frank Becker CLA 2010-02-03 15:20:59 EST
Created attachment 158093 [details]
patch

save of the current sources until the review is done.
Comment 21 Frank Becker CLA 2010-02-03 15:21:03 EST
Created attachment 158094 [details]
mylyn/context/zip
Comment 22 Robert Elves CLA 2010-02-11 11:47:59 EST
There was some discussion last week about pulling the comment field out similarly to the attachment details in this attachment dialog as well. I'll bring it up on today's call.
Comment 23 Robert Elves CLA 2010-02-11 13:30:19 EST
+1 for patch in its current form Frank. After some use we can decide about moving the comment field.
Comment 24 Frank Becker CLA 2010-02-11 14:49:56 EST
Created attachment 158903 [details]
commited patch
Comment 25 Frank Becker CLA 2010-02-11 14:50:05 EST
Created attachment 158904 [details]
mylyn/context/zip
Comment 26 Frank Becker CLA 2010-02-11 14:51:37 EST
(In reply to comment #23)
> +1 for patch in its current form Frank. After some use we can decide about
> moving the comment field.

I close this for now.

If we want to move the comment field we can reopten ore create a new one.