Bug 846660 (mate-dialogs) - Review Request: mate-dialogs -- Display dialog boxes from shell scripts
Summary: Review Request: mate-dialogs -- Display dialog boxes from shell scripts
Status: CLOSED ERRATA
Alias: mate-dialogs
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On: libmatenotify
Blocks: MATE-DE-tracker 844165
TreeView+ depends on / blocked
 
Reported: 2012-08-08 11:26 UTC by Dan Mashal
Modified: 2012-08-22 21:11 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-22 21:07:43 UTC
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
rdieter: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Dan Mashal 2012-08-08 11:26:53 UTC
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
Description: Fork of gdialog. Displays dialog boxes from shell scripts.

Comment 1 Ankur Sinha (FranciscoD) 2012-08-08 11:37:15 UTC
I shall review this.

Comment 2 Rex Dieter 2012-08-10 15:04:55 UTC
Ankur, I can look at this today too (I see you didn't set the  review flag or assign this to yourself yet)

Comment 3 Rex Dieter 2012-08-10 15:17:03 UTC
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)

Comment 4 Ankur Sinha (FranciscoD) 2012-08-11 10:10:15 UTC
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

Comment 5 Rex Dieter 2012-08-11 14:44:19 UTC
ok, consider comment #3 as part of formal review then.  after  that, we're  *real* close.

Comment 6 Wolfgang Ulbrich 2012-08-11 19:30:31 UTC
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.

Comment 7 Wolfgang Ulbrich 2012-08-11 19:32:34 UTC
(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.

Comment 8 Rex Dieter 2012-08-11 20:02:53 UTC
I agree with comment #7, except leave out ", but is intended to be saner"

Comment 9 Dan Mashal 2012-08-11 23:19:17 UTC
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

Comment 10 Dan Mashal 2012-08-11 23:20:47 UTC
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

Comment 11 Dan Mashal 2012-08-11 23:22:02 UTC
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.

Comment 12 Rex Dieter 2012-08-12 00:19:46 UTC
missed item 3,  but close enough. :)


APPROVED

Comment 13 Dan Mashal 2012-08-12 01:21:17 UTC
k will fix that in the official build. Thanks.

Comment 14 Dan Mashal 2012-08-12 01:24:51 UTC
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

Comment 15 Dan Mashal 2012-08-13 02:51:25 UTC
Thanks David, 

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

Comment 16 Gwyn Ciesla 2012-08-13 12:04:02 UTC
Git done (by process-git-requests).

Comment 17 Fedora Update System 2012-08-13 14:36:32 UTC
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

Comment 18 Fedora Update System 2012-08-13 14:36:52 UTC
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

Comment 19 Fedora Update System 2012-08-14 00:59:08 UTC
mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 testing repository.

Comment 20 Fedora Update System 2012-08-22 21:07:43 UTC
mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 stable repository.

Comment 21 Fedora Update System 2012-08-22 21:11:46 UTC
mate-dialogs-1.4.0-2.fc16 has been pushed to the Fedora 16 stable repository.


Note You need to log in before you can comment on or make changes to this bug.