Good work.
Reviewed by: Karel Klic <kklic(a)redhat.com>
On 03/31/2010 10:56 AM, Jiri Moskovcak wrote:
The new patch with all major problems fixed (hopefully).
Please review.
Jirka
On 03/30/2010 05:08 PM, Karel Klic wrote:
> Thank you for the fixes. Please see below.
>
> On 03/30/2010 04:12 PM, Jiri Moskovcak wrote:
>> On 03/30/2010 12:08 PM, Karel Klic wrote:
>>> 2) Help button on the main window shows About dialog.
>>> We have no manual, no help texts. I would hide the button for the
>>> release version to avoid confusion.
>>
>> I think the about dialog is ok (for now), it contains ABRT's url where
>> users can find some info.
>
> So wouldn't it be better to open the web page directly when user clicks
> the button? Several applications do this (pidgin, thunderbird, firefox).
>
- maybe, but for now I'd prefer to leave it that way (or I can rename
the button from "help" to "about" :))
>>> 6) In Details panel uppercase/lowercase is not consistent: compare
>>> "Latest _C_rash" and "Crash _c_ount"
>> - fixed
> The fix seems not to be a part of the attached patches.
>
- fixed now
>>> 7) Main menu -> Help -> About dialog shows two lines:
>>> (C) 2009 Red Hat, Inc.
>>> (C) 2010 Red Hat, Inc.
>>> The copyright lines should not be duplicated for every year.
>>> I would merge them into one line:
>>> (C) 2009, 2010 Red Hat, Inc.
>>
>> - fixed
> The fix seems not to be a part of the attached patches.
>
- fixed
>>
>>> 9) Main menu -> Edit -> Preferences is still visible even when it's
not
>>> implemented. It already confused many users (they reported it as a
>>> bug).
>>> We should hide it at least for official RPMs.
>>>
>> - ok
> The fix seems not to be a part of the attached patches.
>
- I'll add the patch to do this to Fedora cvs and I keep it visible in git
> When all crashes are deleted, the Details panel keeps displaying the
> details of the last crash (see the attached screenshot
> abrt-last-crash-deleted.png).
>
- fixed
> When the GUI is started with no crashes in the database, it shows the
> Details panel (see the attached screenshot abrt-no-crashes-avail.png).
> The panel should be probably hidden, or it can show something like "No
> crash has been detected by ABRT yet." instead of the empty controls.
> Also, the buttons "Delete", "Copy to Clipboard", and
"Report" should be
> disabled (Copy to Clipboard throws an exception, the others do nothing).
>
- fixed
> K.