Bug 846660 (mate-dialogs)

I shall review this.

Ankur, I can look at this today too (I see you didn't set the  review flag or assign this to yourself yet)

well,  some initial comments anyway:

1. MUST remove explicit
Requires: libmatenotify
that dep should get pulled in automatically

2. MUST  fix dir ownership, replace
%{_datadir}/mate/*
%{_datadir}/matedialog/*
with
%{_datadir}/mate/
%{_datadir}/matedialog/
unless there's some better place lower in the stack to own either of these?

3. SHOULD move
NOCONFIGURE=1 ./autogen.sh
to %setup section

4. SHOULD issue a verbose build,  replace
make %{?_smp_mflags}
with
make %{?_smp_mflags} V=1

5. SHOULD(?) remove
Requires:  scrollkeeper
Unless there's some reason to keep it, if so, please document in the .spec why

6. SHOULD remove from  %description:
"Fork of gdialog."  I personally don't think this adds anything of value (end users shouldn't care about it's origins)

Rex,

Please take over the review if you want to. I won't be able to review it in the near future unfortunately. 

Thanks.
Ankur

ok, consider comment #3 as part of formal review then.  after  that, we're  *real* close.

wrong descrition,....... mate dialogs is a fork of zenity.
gdialogs is a binary of zenity, which is remove in zenity to avoid perl deps.
You could also remove this binary in mate-dialogs savely.

(In reply to comment #6)
> wrong descrition,....... mate dialogs is a fork of zenity.
> gdialogs is a binary of zenity, which is remove in zenity to avoid perl deps.
> You could also remove this binary in mate-dialogs savely.

maybe use this
Summary:	Display dialog boxes from shell scripts

%description
mate-dialogs lets you display Gtk+ dialog boxes from the command line and through
shell scripts. It is similar to gdialog, but is intended to be saner. It comes
from the same family as dialog, Xdialog, and cdialog.

I agree with comment #7, except leave out ", but is intended to be saner"

Fixed. Please review.

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-dialogs.spec
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-dialogs-1.4.0-1.fc17.src.rpm

I've temporarily removed libmatenotify-devel so that package will build. I will readd it later.

In regards to comments 7,8 I would rather just keep a short description right now.

Thanks,
Dan

Sorry, forgot to bump the release version in the url:

Spec URL: http://vicodan.fedorapeople.org/matespec/mate-dialogs.spec
SRPM URL: http://vicodan.fedorapeople.org/materpms/srpms/mate-dialogs-1.4.0-2.fc17.src.rpm

I don't know how this got marked as a security response review, and I cannot remove it, just fyi in case anyone's wondering.

missed item 3,  but close enough. :)


APPROVED

k will fix that in the official build. Thanks.

New Package SCM Request
=======================
Package Name: mate-dialogs
Short Description: Displays dialog boxes from shell scripts for MATE Desktop.
Owners: vicodan raveit65 rdieter
Branches: f16 f17 f18

Thanks David, 

Jon, I have gotten the security flag removed. Please process at your earliest convenience.

Git done (by process-git-requests).

mate-dialogs-1.4.0-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/mate-dialogs-1.4.0-2.fc16

mate-dialogs-1.4.0-2.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/mate-dialogs-1.4.0-2.fc17

mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 testing repository.

mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 stable repository.

mate-dialogs-1.4.0-2.fc16 has been pushed to the Fedora 16 stable repository.