Bug 226026 - Merge Review: libgtop2
Merge Review: libgtop2
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 14:24 EST by Nobody's working on this, feel free to take it
Modified: 2010-10-06 15:31 EDT (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-10-05 00:52:08 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)
spec cleanup (2.62 KB, patch)
2010-09-24 05:44 EDT, Parag AN(पराग)
no flags Details | Diff

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:24:52 EST
Fedora Merge Review: libgtop2

http://cvs.fedora.redhat.com/viewcvs/devel/libgtop2/
Initial Owner: sandmann@redhat.com
Comment 1 Marcin Zajaczkowski 2007-03-03 13:20:39 EST
That's my first review of someone's else package so please be understanding and
make your suggestions to my review.

REVIEW:
- rpmlint reports warning (see below)
+ packagename is fine
+ specfile name is fine
+ license GPL, inlcuded in %doc
+ md5sum matches upstream
- BuildRequires - suggestions below
+ locales OK
+ ldconfig in %post and %postun
+ no problems with directories
+ no duplicates in %files
+ %defattr is set
+ %clean section looks good
+ macros are used
+ headres in -devel
+ .la files removed
+ pkgconfig in Requires (due to .pc files)
- not clear build options (see below)


THINGS:

rpmlint warings:
W: libgtop2 summary-not-capitalized libgtop library (version 2)
W: libgtop2 mixed-use-of-spaces-and-tabs (spaces: line 8, tab: line 7)

Summary sections SHOULD be extended.
"libgtop library (version 2)" doesn't say too much for most people.

BuildRequires:
texinfo doesn't seem to be required (in my opinion) and SHOULD be removed. info
file is created without it (and is deleted anyway by a command in spec file).
texinfo depends on several other packages.

gtk-doc package won't be needed in gtk-doc files are not intended (see below)


building:

gtk-doc files are created despite of the option --disable-gtk-doc in configure.
It could be problem with upstream. I'm not sure if gtk-doc are intended or not
because in %files section there is "%{_datadir}/gtk-doc/html/libgtop".
In libgtop2-devel for FC5 there are not that files. In FC7 they are.
When gtk-doc is not available in a system libgtop is built without them, so if
those files are not needed gtk-doc (depends on several other packages) could be
removed from BuildRequires list (if the next point was changed).


In SRPMS there is a patch (libgtop-2.0.2-prog_as.patch) which adds "AM_PROG_AS"
to configure.in. I'm not an expert of automake, but I'm not sure if it's
required, because without it libgtop2 builds just fine. Maybe it was required
only in libgtop-2.0.x series?
This patch has also one side effect. Because it changes configure.in configure
script has to be rebuilt and to do that gtk-doc is required (which wouldn't be
omited if gtk-doc's files are not intended to be in a package).


Btw, I changed status to NEEDINFO from Assignee (should be the owner of a
package), but I'm not sure about that status.
Comment 2 Marcin Zajaczkowski 2007-03-29 09:58:07 EDT
No response for 4 weeks. Needinfo targed changed to (probable) maintainer (who
should get last notification anyway).
Comment 3 Parag AN(पराग) 2010-09-24 05:44:21 EDT
Created attachment 449376 [details]
spec cleanup

mclasen,
    I have removed libs.patch as I don't see any effect of it in existing spec written. Please approve attached patch.
Comment 4 Parag AN(पराग) 2010-10-05 00:52:08 EDT
Removed libs patch.

Applied cleanup patch in build libgtop2-2.28.2-2.fc15 

APPROVED.
Comment 5 Marcin Zajaczkowski 2010-10-06 15:31:12 EDT
I've already lost all hope to see anything in that issue (3,5+ years :) ). Thanks for interest.

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