Community
Participate
Working Groups
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
Created attachment 154859 [details] commited patch
Created attachment 154860 [details] mylyn/context/zip
Steffen, Rob, please review the commited patch.
Created attachment 154903 [details] reverted patch Sorry I have to do some cleanup and first want to wait until the review is finished.
Created attachment 154904 [details] mylyn/context/zip
Created attachment 155782 [details] screen shot
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
Created attachment 156086 [details] desired functionality screen shot Attaching screen shot of UI aspect of desired functionality. --Deepak
(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.
Frank, what is minimum Bugzilla version required for flag support? If it isn't already this should be documented in the Mylyn FAQ somewhere.
(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).
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.
Created attachment 157454 [details] screen shot of new UI
Created attachment 157456 [details] patch for new UI
Created attachment 157457 [details] mylyn/context/zip
Great Frank. Added ui review to today's meeting agenda.
See comment regarding scrolled pane on bug#272207. If you could get these two working consistently with the scrolled pane that would be ideal.
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.
Created attachment 157849 [details] screenshot changed UI with scroller
Created attachment 158093 [details] patch save of the current sources until the review is done.
Created attachment 158094 [details] mylyn/context/zip
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.
+1 for patch in its current form Frank. After some use we can decide about moving the comment field.
Created attachment 158903 [details] commited patch
Created attachment 158904 [details] mylyn/context/zip
(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.