Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
Summary: Merge Review: gnome-system-monitor Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: sandmann@redhat.com
Fedora Merge Review: gnome-system-monitor
http://cvs.fedora.redhat.com/viewcvs/devel/gnome-system-monitor/ Initial Owner: sandmann@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
dakingun@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO AssignedTo|nobody@fedoraproject.org |dakingun@gmail.com Flag| |fedora-review-, needinfo?
------- Additional Comments From dakingun@gmail.com 2007-02-07 16:15 EST ------- 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
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
dakingun@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEEDINFO |ASSIGNED Flag|needinfo? |
------- Additional Comments From dakingun@gmail.com 2007-02-08 07:25 EST ------- Another needswork; g-s-monitor use of %makeinstall macro should be changed to "make DESTDIR=$RPM_BUILD_ROOT install", according to the guildelines @ http://www.fedoraproject.org/wiki/Packaging/Guidelines#head-fcaf3e6fcbd51194...
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
------- Additional Comments From mclasen@redhat.com 2007-02-08 08:27 EST -------
- call to scrollkeeper-update in post section is inclomplete, should be called like; "scrollkeeper-update -q -o %{_datadir}/omf/%{name} || :"
There is no need to slavishly follow the examples in the guidelines to the letter. The current call works fine
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
------- Additional Comments From mclasen@redhat.com 2007-02-08 08:38 EST ------- * The post and postun Requires on desktop-file-utils is not necessary
There is no such requires in my checkout of the spec file ?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
------- Additional Comments From dakingun@gmail.com 2007-02-08 08:46 EST ------- (In reply to comment #4)
- The post and postun Requires on desktop-file-utils is not necessary
There is no such requires in my checkout of the spec file ?
Yeah, it's truly not there, i must have confused it with some other package.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
mclasen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-review- |fedora-review?
------- Additional Comments From mclasen@redhat.com 2007-02-08 08:58 EST ------- * Thu Feb 8 2007 Matthias Clasen mclasen@redhat.com - 2.17.6-2 - Remove an obsolete Obsoletes: - Don't add X-Redhat-Base to the desktop file anymore
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
dakingun@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|dakingun@gmail.com |nobody@fedoraproject.org Flag|fedora-review? |fedora-review+
------- Additional Comments From dakingun@gmail.com 2007-02-08 09:31 EST ------- rpmlint now silent on srpm, and gives warnings (which can be ignored) on the binary [deji@agape reviews]$ rpmlint gnome-system-monitor-2.17.6-2.fc7.x86_64.rpm W: gnome-system-monitor no-documentation W: gnome-system-monitor non-conffile-in-etc /etc/gconf/schemas/gnome-system-monitor.schemas
APPROVED.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
dakingun@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nobody@fedoraproject.org |mclasen@redhat.com
------- Additional Comments From dakingun@gmail.com 2007-02-08 10:16 EST ------- Sorry, the owner actually have a name.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: gnome-system-monitor
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=225838
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora
mclasen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution| |CURRENTRELEASE
------- Additional Comments From mclasen@redhat.com 2007-08-10 21:28 EST ------- This is done.
package-review@lists.fedoraproject.org