Bug 485496 - Review request of geglmm - the C++ Binding to the GEGL library
Summary: Review request of geglmm - the C++ Binding to the GEGL library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Mamoru TASAKA
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-02-13 19:10 UTC by Dodji Seketeli
Modified: 2009-02-18 22:30 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-02-18 22:30:46 UTC
Type: ---
Embargoed:
mtasaka: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Dodji Seketeli 2009-02-13 19:10:03 UTC
spec URL: http://people.redhat.com/dseketel/rpms/geglmm/geglmm.spec-0.0.22-1
SRPM URL: http://people.redhat.com/dseketel/rpms/geglmm/geglmm-0.0.22-1.src.rpm

Description:

This is the C++ binding of the GEGL graph based image processing framework.
This bindings adhere to the APIs of the gtkmm2 library.

I have put the result of rpmlint at http://people.redhat.com/dseketel/rpms/geglmm/rpmlint-output-geglmm.spec-0.0.22-1.txt.

It's spitting an only-non-binary-in-usr-lib error, and I think it's because of the things that are being install in %{_libdir}, like the m4 files, the pkgconfig files and the configuration header files. Those files are necessary, I think. Isn't rpmlint being a bit too pedantic ?

Comment 1 Mamoru TASAKA 2009-02-14 17:33:40 UTC
(In reply to comment #0)
> It's spitting an only-non-binary-in-usr-lib error, and I think it's because of
> the things that are being install in %{_libdir}, like the m4 files, the
> pkgconfig files and the configuration header files. 

+ rpmlint won't complain for pkgconfig file as only-non-binary-in-usr-lib,
  however for m4 files and header file rpmlint currently complains.
  For this package this rpmlint can be ignored.

Then:

Some random notes:

- Please consider to use %{?dist} macro:
  https://fedoraproject.org/wiki/Packaging/DistTag

- Source0 must be given with full URL:
  https://fedoraproject.org/wiki/Packaging/SourceURL

- gegl-devel Requires babl-devel, so "BuildRequires: babl-devel"
  is redundant.

- Usually the dependencies between binary rpms rebuilt from
  a srpm must be EVR (not just version) specific.
  https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

- This srpm won't build on dist-f11:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=1126647

- We recommend %defattr(-,root,root,-)

- Please consider to add the following files
------------------------------------------------------------
AUTHORS
COPYING
COPYING.LESSER
------------------------------------------------------------
  to main package and
------------------------------------------------------------
ChangeLog
------------------------------------------------------------
  to -devel package.

Comment 2 Dodji Seketeli 2009-02-15 15:23:02 UTC
{?dist} macro:
>   https://fedoraproject.org/wiki/Packaging/DistTag

Done.
 
> - Source0 must be given with full URL:
>   https://fedoraproject.org/wiki/Packaging/SourceURL

Done.

> 
> - gegl-devel Requires babl-devel, so "BuildRequires: babl-devel"
>   is redundant.

I Removed babl-devel.
 
> - Usually the dependencies between binary rpms rebuilt from
>   a srpm must be EVR (not just version) specific.
>   https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

Done.

> - This srpm won't build on dist-f11:
>   http://koji.fedoraproject.org/koji/taskinfo?taskID=1126647
>

Ah. It builds on f10 that's why I didn't notice that. I will now use mock.
So I did a patch to fix this issue. The patch has been submitted upstream.
While at it, I produced another patchlet to kill some warnings. That patch has been submitted upstream as well.
 
> - We recommend %defattr(-,root,root,-)

Done.

> - Please consider to add the following files
> ------------------------------------------------------------
> AUTHORS
> COPYING
> COPYING.LESSER
> ------------------------------------------------------------
>   to main package and
> ------------------------------------------------------------
> ChangeLog
> ------------------------------------------------------------
>   to -devel package.
> 

Done.

Thanks for the quick review.

Comment 3 Dodji Seketeli 2009-02-15 15:26:55 UTC
I forgot to say that a new srpm and spec are available at:
SRPM: http://people.redhat.com/~dseketel/rpms/geglmm/geglmm-0.0.22-2.fc10.src.rpm
SPEC: http://people.redhat.com/~dseketel/rpms/geglmm/geglmm.spec-0.0.22-2

Comment 4 Mamoru TASAKA 2009-02-15 16:11:22 UTC
Okay.

- I recommend to use %{version} in Source0
  ref:
  https://fedoraproject.org/wiki/Packaging/SourceURL#Using_.25.7Bversion.7D
  If you want to change, please fix this when importing into
  Fedora CVS.

-------------------------------------------------------------------
    This package (geglmm) is APPROVED by mtasaka
-------------------------------------------------------------------

Please follow the procedure written on:
http://fedoraproject.org/wiki/PackageMaintainers/Join
from "Install the Client Tools (Koji)".

Now I am sponsoring you.

If you want to import this package into Fedora 9/10, you also have
to look at
http://fedoraproject.org/wiki/Infrastructure/UpdatesSystem/Bodhi-info-DRAFT
(after once you rebuilt this package on koji Fedora rebuilding system).

If you have questions, please ask me.

Removing NEEDSPONSOR.

Comment 5 Dodji Seketeli 2009-02-15 17:21:56 UTC
Thanks.

Comment 6 Dodji Seketeli 2009-02-15 17:23:16 UTC
New Package CVS Request
=======================
Package Name: geglmm
Short Description:  A graphic processing library, C++ bindings
Owners: dodji
Branches: F-10
InitialCC: dodji

Comment 7 Kevin Fenzi 2009-02-16 21:35:03 UTC
cvs done.

Comment 8 Mamoru TASAKA 2009-02-18 17:13:52 UTC
Please rebuild this package also on F-10 branch, and
for F-10 submit a request to push the rebuilt package into 
repository on bodhi:
https://admin.fedoraproject.org/updates/

When it is done, please close this bug as NEXTRELEASE.


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