[Bug 666409] Review Request: t4k_common - Library for Tux4Kids applications

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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

Martin Gieseking <martin.gieseking@xxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |martin.gieseking@xxxxxx

--- Comment #3 from Martin Gieseking <martin.gieseking@xxxxxx> 2011-02-23 09:56:52 EST ---
(In reply to comment #1)
> [+] Meet the Packaging Guidelines
>     *** NOTE: no longer need %clean/cleaning of the buildroot in %install
> unless building for F12 and below  or EPEL   

Right. F12 has reached end of life, so we don't have to care about it any
longer. The buildroot stuff is still required for EPEL < 6. Jon, if you intend
to keep it, please adapt the BuildRoot field according to
http://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#BuildRoot_tag


> [+] The sources used to build the package must match the upstream source

That's OK. Brendan, please copy the md5sums of the tarballs into your reviews
so that we can easily verify the identity. 

$ md5sum t4k_common-0.0.3.tar.gz*
28ad0818aa79d701fd33019e756340f8  t4k_common-0.0.3.tar.gz
28ad0818aa79d701fd33019e756340f8  t4k_common-0.0.3.tar.gz.upstream


> [!] A package must own all directories that it creates
>     *** line 54: must own %{_datadir}/%{name} - do not need to qualify files or
> directories under this

Yes, %{_datadir}/%{name} is currently unowned. Either replace 
%{_datadir}/%{name}/images with %{_datadir}/%{name}/ 
or add %dir %{_datadir}/%{name}


> [+] Permissions on files must be set properly. Every %files section must
> include a %defattr(...) line

The %defattrs should look like this: %defattr(-,root,root,-)


> [!] Each package must consistently use macros
>     *** use %{name} macro in Source0

OK, but using %{name} in Source0 is optional.


Some additional notes:
- you should preserve the timestamps by adding INSTALL='install -p' to
  "make install"

- drop INSTALL as it's not of much use in a binary package

- move README to the base package, and also add file ChangeLog

- please be more specific in %files especially when only single files/folders 
  are added, e.g.:
  %{_libdir}/*.so.* => %{_libdir}/lib%{name}.so.*
  %{_libdir}/*.so   => %{_libdir}/lib%{name}.so
  %{_includedir}/*  => %{_includedir}/%{name}.h

- if you want to use the %{name} macro in the %files section, please add the
  .pc file with %{_libdir}/pkgconfig/%{name}.pc

- I suggest to also build the API docs (with doxygen) and add the HTML variant 
  to the -devel package

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review


[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]