[Bug 182463] Review Request: cairomm (C++ bindings for cairo)

[bugzilla at 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: Review Request: cairomm (C++ bindings for cairo)


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=182463


seg at haxxed.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |seg at haxxed.com




------- Additional Comments From seg at haxxed.com  2006-03-06 01:13 EST -------
I can't sponsor, but here's my first review ever:

MUST items:

- rpmlint: ?
E: cairomm shlib-with-non-pic-code /usr/lib/libcairomm-1.0.so.0.0.5

I don't know how big a problem this is or how to fix it. (FC5t3 i386)

- Package name: Ok
- Spec name: Ok
- Meets packaging guidelines: Ok
- License: Ok
- Spec in American English: Ok
- Spec legible: Ok
- Sources match upstream: Ok
- Builds: Yes (FC5t3 i386)
- BuildRequires: Ok
- Locales: Ok
- ldconfig: Ok
- Relocation: Ok
- Directory ownership: Ok
- %files: Ok
- %clean: Ok
- Macros: Ok
- Code vs. Content: Ok
- Documentation: Ok
- devel package: Ok
- .desktop file: n/a

SHOULD:

- Includes license text: Ok
- Spec translations: Ok?
- Mock build: n/a, I don't have mock set up yet
- Builds on all archs: n/a, I only have i386
- Package functional: Ok, included png_file example compiles and runs
- Scriptlets: Ok
- Subpackages: Ok

NEEDSWORK:

There are duplicate Group: lines in the devel package.

I notice the devel package installs the API reference into
/usr/share/doc/libcairomm-1.0, upstream seems confused as to what the
name/verison really is. This is probably something to bug upstream about. For
now, I'd see if you can use configure flags to convince it to put reference/
into /usr/share/doc/cairomm-%{version}/ instead, failing that, move it over in
%install

Style soapbox (These aren't blockers):

IMHO, macros are for versions and paths. Stuff that can and will change. I don't
like the over use of %{name} everywhere. Especially in %description and Summary.
IMHO, %{name} should be used in BuildRoot and nowhere else. Renaming a package
shouldn't happen very often, so there's little need for using an ugly macro.

Also, I would specify %files a bit more exactly. I find its best to be aware of
when upstream adds/renames/removes files, excessive globbing can end up with
mysterious new files turning up in the wrong package. Better to error out due to
unpackaged files. I would use:

%files
%defattr(-,root,root,-)
%doc AUTHORS COPYING README NEWS ChangeLog
%{_libdir}/libcairomm-1.0.so.0*

%files devel
%defattr(-,root,root,-)
%{_libdir}/libcairomm-1.0.so
%{_libdir}/pkgconfig/cairomm-1.0.pc
%{_includedir}/cairomm-1.0/
%doc %{_datadir}/doc/libcairomm-1.0/

-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.