Bug 846660 (mate-dialogs)

Summary: Review Request: mate-dialogs -- Display dialog boxes from shell scripts
Product: [Fedora] Fedora Reporter: Dan Mashal <dan.mashal>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: djorm, dwmw2, notting, package-review, rdieter
Target Milestone: ---Flags: rdieter: fedora‑review+
limburgher: fedora‑cvs+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-22 17:07:43 EDT Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 846661    
Bug Blocks: 840149, 844165    

Description Dan Mashal 2012-08-08 07:26:53 EDT
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 07:37:15 EDT
I shall review this.
Comment 2 Rex Dieter 2012-08-10 11:04:55 EDT
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 11:17:03 EDT
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 06:10:15 EDT
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 10:44:19 EDT
ok, consider comment #3 as part of formal review then.  after  that, we're  *real* close.
Comment 6 Wolfgang Ulbrich 2012-08-11 15:30:31 EDT
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 15:32:34 EDT
(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 16:02:53 EDT
I agree with comment #7, except leave out ", but is intended to be saner"
Comment 9 Dan Mashal 2012-08-11 19:19:17 EDT
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 19:20:47 EDT
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 19:22:02 EDT
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-11 20:19:46 EDT
missed item 3,  but close enough. :)


APPROVED
Comment 13 Dan Mashal 2012-08-11 21:21:17 EDT
k will fix that in the official build. Thanks.
Comment 14 Dan Mashal 2012-08-11 21:24:51 EDT
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-12 22:51:25 EDT
Thanks David, 

Jon, I have gotten the security flag removed. Please process at your earliest convenience.
Comment 16 Jon Ciesla 2012-08-13 08:04:02 EDT
Git done (by process-git-requests).
Comment 17 Fedora Update System 2012-08-13 10:36:32 EDT
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 10:36:52 EDT
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-13 20:59:08 EDT
mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 testing repository.
Comment 20 Fedora Update System 2012-08-22 17:07:43 EDT
mate-dialogs-1.4.0-2.fc17 has been pushed to the Fedora 17 stable repository.
Comment 21 Fedora Update System 2012-08-22 17:11:46 EDT
mate-dialogs-1.4.0-2.fc16 has been pushed to the Fedora 16 stable repository.