fedora-review denied: [Bug 226513] Merge Review: units
by Red Hat Bugzilla
Bug 226513: Merge Review: units
Product: Fedora Extras
Version: devel
Component: Package Review
Kevin Fenzi <kevin(a)tummy.com> has denied Kevin Fenzi <kevin(a)tummy.com>'s
request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226513
------- Additional Comments from Kevin Fenzi <kevin(a)tummy.com>
OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
e27f580474702e9138b332acbafafe5b units-1.86.tar.gz
e27f580474702e9138b332acbafafe5b units-1.86.tar.gz.1
See below - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
See below - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:
SHOULD Items:
OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version
0 outstanding bugs - check for outstanding bugs on package.
Issues:
1. Might include in %doc:
ChangeLog COPYING NEWS README
2. Buildroot should be the default one:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
3. rpmlint says:
rpmlint on units-1.86-3.fc7.src.rpm
W: units summary-ended-with-dot A utility for converting amounts from one unit
to another.
Suggest: remove . at the end of summary.
W: units no-url-tag
Suggest: add a "URL: http://www.gnu.org/software/units/units.html"
W: units prereq-use /sbin/install-info
Suggest: remove the "Prereq: /sbin/install-info" and replace per
http://www.fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-47896d...
62d75deefeb9ba75145a398515db
with
Requires(post): /sbin/install-info
Requires(preun): /sbin/install-info
4. Don't use %makeinstall, instead use 'make DESTDIR=$RPM_BUILD_ROOT install'
per
http://www.fedoraproject.org/wiki/PackagingDrafts/MakeInstall
5. Possible missing Buildrequires?
checking for sin... no
checking for tparm in -lncurses... no
checking for tgetent in -ltermcap... no
checking for readline in -lreadline... no
Once you have addressed these items (either by making the suggested changes, or
by explaining why they don't make sense), please reassign this review back
to me, and change the 'fedora-review' flag back to ? for me to take action.
17 years, 3 months
fedora-review granted: [Bug 226660] Merge Review: xterm
by Red Hat Bugzilla
Bug 226660: Merge Review: xterm
Product: Fedora Extras
Version: devel
Component: Package Review
Kevin Fenzi <kevin(a)tummy.com> has granted Miroslav Lichvar
<mlichvar(a)redhat.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226660
------- Additional Comments from Kevin Fenzi <kevin(a)tummy.com>
Thanks for the prompt fixes.
Just to record here, we discussed adding the desktop file today in the
#fedora-desktop irc channel. It seems it should be fine to do so.
If this turns out to be a problem due to xterm being installed for the gdm
failsafe mode, we can always go back and remove the desktop file.
I see no further blockers here, so this package is APPROVED.
I think the current plan is to leave this review assigned to you,
and with the fedora-review flag set to + until it can be determined how to
set them up in CVS.
Thanks again.
17 years, 3 months
fedora-review denied: [Bug 225237] Merge Review: acpid
by Red Hat Bugzilla
Bug 225237: Merge Review: acpid
Product: Fedora Extras
Version: devel
Component: Package Review
Kevin Fenzi <kevin(a)tummy.com> has denied Kevin Fenzi <kevin(a)tummy.com>'s
request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225237
------- Additional Comments from Kevin Fenzi <kevin(a)tummy.com>
Thanks for the quick fixes and reply...
1. good. ok.
2. good. ok.
3.1 good. ok.
3.2 I suppose so. ok.
3.3 good. ok.
3.4 Not sure why it's complaining about that 0755 init script. ;(
good on the prereqs.
3.5 good. ok.
4.1 Humm...I haven't seen that used for /var/log/FOO files before... I guess
there aren't many packages that make such files. Looking at for example
vixie-cron, it just has no mention of the log file at all.
I guess the question is: do we want this package to remove the /var/log/acpid
log when the package is removed? Or should it linger around?
We should probibly decide what behavior is desired and make all the packages
that create /var/log files do the same thing...
In fact I am hard pressed to find another package that does things the way
this one does. Do you have any example? scrollkeeper appears to ghost the log
file, but yum, x.org, vixie-cron just don't claim anything for their log file
(ie, it's unowned).
4.2 good. ok.
4.3 good. ok.
Setting it back to you to comment on the 4.1 issue.
When you do, please assign it back to me and change the fedora-review flag back
to ?
Thanks again for the quick fixes...
17 years, 3 months
fedora-review granted: [Bug 225609] Merge Review: bash
by Red Hat Bugzilla
Bug 225609: Merge Review: bash
Product: Fedora Extras
Version: devel
Component: Package Review
Kevin Fenzi <kevin(a)tummy.com> has granted Tim Waugh <twaugh(a)redhat.com>'s
request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225609
------- Additional Comments from Kevin Fenzi <kevin(a)tummy.com>
Excellent. I see no further blockers, so this package is APPROVED.
Thanks for the great responsiveness and fixes...
I think the plan is to leave these review bugs around assigned to you and in
the
fedora-review + state until it's determined how to merge them into CVS.
17 years, 3 months
fedora-review denied: [Bug 225822] Merge Review: gnome-media
by Red Hat Bugzilla
Bug 225822: Merge Review: gnome-media
Product: Fedora Extras
Version: devel
Component: Package Review
Deji Akingunola <dakingun(a)gmail.com> has denied Deji Akingunola
<dakingun(a)gmail.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225822
------- Additional Comments from Deji Akingunola <dakingun(a)gmail.com>
A couple of issues for gnome-media;
NEEDSWORK:
* rpmlint (on the binary package, the -devel subpackage, and the srpm)
produces
a number of warnings/errors. Most of it can possibly be ignored, but I think
the
rpath issue need to be fixed
>>
[deji@agape reviews]$ rpmlint gnome-media-2.17.90-1.fc7.x86_64.rpm
E: gnome-media tag-not-utf8 %changelog
E: gnome-media obsolete-not-provided gnome
E: gnome-media binary-or-shlib-defines-rpath
/usr/lib64/libglade/2.0/libgnome-media-profiles.so ['/usr/lib64']
E: gnome-media binary-or-shlib-defines-rpath
/usr/bin/gnome-audio-profiles-properties ['/usr/lib64']
E: gnome-media binary-or-shlib-defines-rpath /usr/bin/gnome-cd ['/usr/lib64']
E: gnome-media binary-or-shlib-defines-rpath /usr/libexec/cddb-track-editor
['/usr/lib64']
W: gnome-media non-conffile-in-etc /etc/gconf/schemas/CDDB-Slave2.schemas
W: gnome-media non-conffile-in-etc
/etc/gconf/schemas/gnome-audio-profiles.schemas
W: gnome-media non-conffile-in-etc
/etc/gconf/schemas/gnome-volume-control.schemas
W: gnome-media non-conffile-in-etc /etc/gconf/schemas/gnome-cd.schemas
[deji@agape reviews]$ rpmlint gnome-media-devel-2.17.90-1.fc7.x86_64.rpm
E: gnome-media-devel tag-not-utf8 %changelog
W: gnome-media-devel no-documentation
[deji@agape reviews]$ rpmlint gnome-media-2.17.90-1.fc7.src.rpm
E: gnome-media tag-not-utf8 %changelog
E: gnome-media non-utf8-spec-file gnome-media.spec
W: gnome-media unversioned-explicit-obsoletes gnome
W: gnome-media macro-in-%changelog _datadir
W: gnome-media mixed-use-of-spaces-and-tabs (spaces: line 12, tab: line 22)
* scrollkeeper-update is not being properly called in the post and postun
sections according to
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?action=show&red...
ScriptletSnippets#head-a4ea5e1946bc113d19d24b4f5bfb543c579e5fc8
* "--add-category X-Redhat-Base" option to desktop-file-install is no longer
necesary
Not so importantly, the "disable-schemas-install" option can be pass to the
configure script instead of setting and unsetting
GCONF_DISABLE_MAKEFILE_SCHEMA_INSTAL env.
17 years, 3 months
fedora-review denied: [Bug 225838] Merge Review: gnome-system-monitor
by Red Hat Bugzilla
Bug 225838: Merge Review: gnome-system-monitor
Product: Fedora Extras
Version: devel
Component: Package Review
Deji Akingunola <dakingun(a)gmail.com> has denied Deji Akingunola
<dakingun(a)gmail.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
------- Additional Comments from Deji Akingunola <dakingun(a)gmail.com>
Good:
* Package name conforms to the Fedora Naming Guidelines
* Sources, Group and License tags properly in place
* Locales handled correctly
* All necessary BuildRequires listed.
* Package builds OK in Mock.
NEEDSWORK:
According to the packaging guildlines on rpm scriplets,
* The post and postun Requires on desktop-file-utils is not necessary
* call to scrollkeeper-update in post section is inclomplete, should be called
like;
"scrollkeeper-update -q -o %{_datadir}/omf/%{name} || :"
* I believe the "add-category X-Redhat-Base" is no longer necessary for the
desktop file install.
A couple of other minor nitpicks;
* You can pass the "--disable-schemas-install" option to the configure script,
without needing to set and unset the GCONF_DISABLE_MAKEFILE_SCHEMA_INSTALL env.
* There are a number of rpmlint warnings/errors which I believe can be ignored;
[deji@agape reviews]$ rpmlint gnome-system-monitor-2.17.6-1.fc7.src.rpm
W: gnome-system-monitor unversioned-explicit-obsoletes gtop
[deji@agape reviews]$ rpmlint gnome-system-monitor-2.17.6-1.fc7.x86_64.rpm
E: gnome-system-monitor obsolete-not-provided gtop
W: gnome-system-monitor no-documentation
W: gnome-system-monitor non-conffile-in-etc
/etc/gconf/schemas/gnome-system-monitor.schemas
17 years, 3 months